From f9501e97a2bb1c514c61ea5b5fc58d845748470d Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Mon, 2 Jan 2017 14:14:51 +0100 Subject: [PATCH] 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 --- src/restic/repository/master_index.go | 13 +++++-------- src/restic/repository/packer_manager.go | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/restic/repository/master_index.go b/src/restic/repository/master_index.go index b037ac23f..165bf6024 100644 --- a/src/restic/repository/master_index.go +++ b/src/restic/repository/master_index.go @@ -119,16 +119,14 @@ func (mi *MasterIndex) Remove(index *Index) { } } -// Current returns an index that is not yet finalized, so that new entries can -// still be added. If all indexes are finalized, a new index is created and -// returned. -func (mi *MasterIndex) Current() *Index { +// Store remembers the id and pack in the index. +func (mi *MasterIndex) Store(pb restic.PackedBlob) { mi.idxMutex.RLock() - for _, idx := range mi.idx { if !idx.Final() { mi.idxMutex.RUnlock() - return idx + idx.Store(pb) + return } } @@ -137,9 +135,8 @@ func (mi *MasterIndex) Current() *Index { defer mi.idxMutex.Unlock() newIdx := NewIndex() + newIdx.Store(pb) mi.idx = append(mi.idx, newIdx) - - return newIdx } // NotFinalIndexes returns all indexes that have not yet been saved. diff --git a/src/restic/repository/packer_manager.go b/src/restic/repository/packer_manager.go index 499abd90b..c686effe6 100644 --- a/src/restic/repository/packer_manager.go +++ b/src/restic/repository/packer_manager.go @@ -133,7 +133,7 @@ func (r *Repository) savePacker(p *pack.Packer) error { // update blobs in the index for _, b := range p.Blobs() { 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{ Type: b.Type, ID: b.ID,