From 4bd1ce868f4ea4b76a378a9efa33d7a4255e26d4 Mon Sep 17 00:00:00 2001 From: Egon Elbre Date: Mon, 23 Sep 2019 22:14:39 +0300 Subject: [PATCH] satellite/metainfo: close loop separately to avoid logical races (#3100) --- satellite/metainfo/loop.go | 8 ++++++-- satellite/metainfo/loop_test.go | 4 ++++ satellite/peer.go | 9 ++++++--- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/satellite/metainfo/loop.go b/satellite/metainfo/loop.go index 5cd50a738..5f59b780c 100644 --- a/satellite/metainfo/loop.go +++ b/satellite/metainfo/loop.go @@ -121,8 +121,6 @@ func (loop *Loop) Join(ctx context.Context, observer Observer) (err error) { func (loop *Loop) Run(ctx context.Context) (err error) { defer mon.Task()(&ctx)(&err) - defer close(loop.done) - for { err := loop.runOnce(ctx) if err != nil { @@ -131,6 +129,12 @@ func (loop *Loop) Run(ctx context.Context) (err error) { } } +// Close closes the looping services. +func (loop *Loop) Close() (err error) { + close(loop.done) + return nil +} + // runOnce goes through metainfo one time and sends information to observers. func (loop *Loop) runOnce(ctx context.Context) (err error) { defer mon.Task()(&ctx)(&err) diff --git a/satellite/metainfo/loop_test.go b/satellite/metainfo/loop_test.go index f2b3db93f..ab9d760e6 100644 --- a/satellite/metainfo/loop_test.go +++ b/satellite/metainfo/loop_test.go @@ -223,6 +223,7 @@ func TestLoopCancel(t *testing.T) { metaLoop := metainfo.NewLoop(metainfo.LoopConfig{ CoalesceDuration: 1 * time.Second, }, satellite.Metainfo.Service) + // create a cancelable context to pass into metaLoop.Run loopCtx, cancel := context.WithCancel(ctx) @@ -270,6 +271,9 @@ func TestLoopCancel(t *testing.T) { err := group.Wait() require.NoError(t, err) + err = metaLoop.Close() + require.NoError(t, err) + obs3 := newTestObserver(nil) err = metaLoop.Join(ctx, obs3) require.Error(t, err) diff --git a/satellite/peer.go b/satellite/peer.go index 2b5673384..5c34a991a 100644 --- a/satellite/peer.go +++ b/satellite/peer.go @@ -670,6 +670,9 @@ func (peer *Peer) Run(ctx context.Context) (err error) { group, ctx := errgroup.WithContext(ctx) + group.Go(func() error { + return errs2.IgnoreCanceled(peer.Metainfo.Loop.Run(ctx)) + }) group.Go(func() error { return errs2.IgnoreCanceled(peer.Version.Run(ctx)) }) @@ -679,9 +682,6 @@ func (peer *Peer) Run(ctx context.Context) (err error) { group.Go(func() error { return errs2.IgnoreCanceled(peer.Repair.Checker.Run(ctx)) }) - group.Go(func() error { - return errs2.IgnoreCanceled(peer.Metainfo.Loop.Run(ctx)) - }) group.Go(func() error { return errs2.IgnoreCanceled(peer.Repair.Repairer.Run(ctx)) }) @@ -788,6 +788,9 @@ func (peer *Peer) Close() error { if peer.Overlay.Service != nil { errlist.Add(peer.Overlay.Service.Close()) } + if peer.Metainfo.Loop != nil { + errlist.Add(peer.Metainfo.Loop.Close()) + } return errlist.Err() }