From 0679e992c728ee3398bfc987f9ee67495acf26fb Mon Sep 17 00:00:00 2001 From: Marvin Preuss Date: Tue, 17 May 2022 13:28:58 +0200 Subject: [PATCH] fix: try to avoid potential race condition in some situations it could try to write to a logger that doesnt exist anymore. no logging but better than an race condition. --- workgroups.go | 2 +- workgroups_test.go | 43 ------------------------------------------- 2 files changed, 1 insertion(+), 44 deletions(-) diff --git a/workgroups.go b/workgroups.go index fec3cd3..0ba661f 100644 --- a/workgroups.go +++ b/workgroups.go @@ -84,7 +84,7 @@ func (d *Dispatcher) Start() { err := j.work(j.ctx) select { case <-j.ctx.Done(): - d.log.V(1).Info("received job return after canceled context", "worker", i, "return", err) + return default: errChan <- err } diff --git a/workgroups_test.go b/workgroups_test.go index 33dcde8..292472e 100644 --- a/workgroups_test.go +++ b/workgroups_test.go @@ -2,7 +2,6 @@ package workgroups_test import ( - "bytes" "context" "errors" "fmt" @@ -15,7 +14,6 @@ import ( "github.com/go-logr/stdr" "github.com/stretchr/testify/require" - "github.com/tonglil/buflogr" "go.xsfx.dev/workgroups" ) @@ -195,44 +193,3 @@ func TestRetry(t *testing.T) { }) } } - -func TestErrChanNotUsed(t *testing.T) { - var buf bytes.Buffer - log := buflogr.NewWithBuffer(&buf) - require := require.New(t) - work := func(ctx context.Context) error { - time.Sleep(5 * time.Second) - - return nil - } - - ctx, cancel := context.WithCancel(context.Background()) - d, ctx := workgroups.NewDispatcher(ctx, log, runtime.GOMAXPROCS(0), 1) - d.Start() - d.Append(workgroups.NewJob(ctx, work)) - d.Close() - - go func() { - time.Sleep(time.Second / 2) - cancel() - }() - - err := d.Wait() - require.ErrorIs(err, context.Canceled) - - time.Sleep(10 * time.Second) - - // Breaking glass! - s := log.GetSink() - - underlier, ok := s.(buflogr.Underlier) - if !ok { - t.FailNow() - } - - bl := underlier.GetUnderlying() - - bl.Mutex().Lock() - require.Contains(buf.String(), "received job return after canceled context") - bl.Mutex().Unlock() -}