KurlesHS/restic
1
0
Fork 0
forked from TrueCloudLab/restic
Code Pull requests Activity

archiver: fix race condition during worker startup

Browse source 
When the tomb is created with a canceled context, then the workers
started via `t.Go` exist nearly immediately. Once for the first time all
started goroutines have been stopped, it is not allowed to issue further
calls to `t.Go`. This is a problem when the started goroutines exit
immediately, as for example the first goroutine might already have
stopped before starting the second one, which is not allowed as once the
first goroutines has stopped no goroutines were running.

To fix this race condition the startup and main task of the archiver now
also run within a `t.Go` function. This also allows unifying the error
handling as it is no longer necessary to distinguish between errors
returned by the workers or the saveTree processing. The tomb now just
returns the first error encountered, which should also be the most
descriptive one.
This commit is contained in:
Michael Eischer 2020-12-30 17:31:22 +01:00
parent b2efa0af39
commit debc4a3a99
1 changed files with 15 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
internal/archiver/archiver.go
View file

@ -784,33 +784,31 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps
var t tomb.Tomb
wctx := t.Context(ctx)
arch.runWorkers(wctx, &t)
start := time.Now()
debug.Log("starting snapshot")
rootTreeID, stats, err := func() (restic.ID, ItemStats, error) {
var rootTreeID restic.ID
var stats ItemStats
t.Go(func() error {
arch.runWorkers(wctx, &t)
debug.Log("starting snapshot")
tree, err := arch.SaveTree(wctx, "/", atree, arch.loadParentTree(wctx, opts.ParentSnapshot))
if err != nil {
return restic.ID{}, ItemStats{}, err
return err
}
if len(tree.Nodes) == 0 {
return restic.ID{}, ItemStats{}, errors.New("snapshot is empty")
return errors.New("snapshot is empty")
}
return arch.saveTree(wctx, tree)
}()
debug.Log("saved tree, error: %v", err)
rootTreeID, stats, err = arch.saveTree(wctx, tree)
// trigger shutdown but don't set an error
t.Kill(nil)
return err
})
t.Kill(nil)
werr := t.Wait()
debug.Log("err is %v, werr is %v", err, werr)
// Use werr when it might contain a more specific error than "context canceled"
if err == nil || (errors.Cause(err) == context.Canceled && werr != nil) {
err = werr
}
err = t.Wait()
debug.Log("err is %v", err)
if err != nil {
debug.Log("error while saving tree: %v", err)