Only add entries to indexes inside PackerManager

This was a nasty bug. Users reported that restic aborts with panic:

    panic: store new item in finalized index

The code calling panic() is in the Store() method of an index and guards
the failure case that an index is to be modified while it has already
been saved in the repo.

What happens here (at least that's what I suspect): PackerManager calls
Current() on a MasterIndex, which yields one index A. Concurrently,
another goroutine calls Repository.SaveFullIndex(), which in turn calls
MasterIndex.FullIndexes(), which (among others) yields the index A. Then
all indexes are marked as final. Then the other goroutine is executed
which adds an entry to the index A, which is now marked as final. Then
the panic occurs.

The commit solves this by removing MasterIndex.Current() and adding a
Store() method that stores the entry in one non-finalized index. This
method uses the same RWMutex as the other methods (e.g. FullIndexes()),
thereby ensuring that the full indexes can only be processed before or
after Store() is called.

Closes #367
This commit is contained in:
Alexander Neumann 2017-01-02 14:14:51 +01:00
parent 3ef788765a
commit f9501e97a2
2 changed files with 6 additions and 9 deletions

View file

@ -119,16 +119,14 @@ func (mi *MasterIndex) Remove(index *Index) {
} }
} }
// Current returns an index that is not yet finalized, so that new entries can // Store remembers the id and pack in the index.
// still be added. If all indexes are finalized, a new index is created and func (mi *MasterIndex) Store(pb restic.PackedBlob) {
// returned.
func (mi *MasterIndex) Current() *Index {
mi.idxMutex.RLock() mi.idxMutex.RLock()
for _, idx := range mi.idx { for _, idx := range mi.idx {
if !idx.Final() { if !idx.Final() {
mi.idxMutex.RUnlock() mi.idxMutex.RUnlock()
return idx idx.Store(pb)
return
} }
} }
@ -137,9 +135,8 @@ func (mi *MasterIndex) Current() *Index {
defer mi.idxMutex.Unlock() defer mi.idxMutex.Unlock()
newIdx := NewIndex() newIdx := NewIndex()
newIdx.Store(pb)
mi.idx = append(mi.idx, newIdx) mi.idx = append(mi.idx, newIdx)
return newIdx
} }
// NotFinalIndexes returns all indexes that have not yet been saved. // NotFinalIndexes returns all indexes that have not yet been saved.

View file

@ -133,7 +133,7 @@ func (r *Repository) savePacker(p *pack.Packer) error {
// update blobs in the index // update blobs in the index
for _, b := range p.Blobs() { for _, b := range p.Blobs() {
debug.Log(" updating blob %v to pack %v", b.ID.Str(), id.Str()) debug.Log(" updating blob %v to pack %v", b.ID.Str(), id.Str())
r.idx.Current().Store(restic.PackedBlob{ r.idx.Store(restic.PackedBlob{
Blob: restic.Blob{ Blob: restic.Blob{
Type: b.Type, Type: b.Type,
ID: b.ID, ID: b.ID,