Skip to content

Commit

Permalink
Fix data race in context wrapper library (#1546)
Browse files Browse the repository at this point in the history
  • Loading branch information
mcastorina authored Jul 25, 2023
1 parent 1a1977f commit 91cbca9
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
9 changes: 7 additions & 2 deletions pkg/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"runtime/debug"
"sync"
"time"

"github.com/go-logr/logr"
Expand All @@ -28,8 +29,9 @@ type CancelFunc = context.CancelFunc
type logCtx struct {
// Embed context.Context to get all methods for free.
context.Context
log logr.Logger
err *error
log logr.Logger
err *error
errLock *sync.Mutex
}

// Logger returns a structured logger.
Expand Down Expand Up @@ -145,8 +147,11 @@ func captureCancelCallstack(ctx logCtx, f context.CancelFunc) (Context, context.
if ctx.err == nil {
var err error
ctx.err = &err
ctx.errLock = &sync.Mutex{}
}
return ctx, func() {
ctx.errLock.Lock()
defer ctx.errLock.Unlock()
// We must check Err() before calling f() since f() sets the error.
// If there's already an error, do nothing special.
if ctx.Err() != nil {
Expand Down
6 changes: 6 additions & 0 deletions pkg/context/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,9 @@ func TestErrCallstackTimeoutCancel(t *testing.T) {
cancel()
assert.Equal(t, err, ctx.Err())
}

func TestRace(t *testing.T) {
_, cancel := WithCancel(Background())
go cancel()
cancel()
}

0 comments on commit 91cbca9

Please sign in to comment.