mirror of
https://github.com/golang/go.git
synced 2025-10-19 11:03:18 +00:00
[release-branch.go1.25] context: don't return a non-nil from Err before Done is closed
The Context.Err documentation states that it returns nil if the
context's done channel is not closed. Fix a race condition introduced
by CL 653795 where Err could return a non-nil error slightly before
the Done channel is closed.
No impact on Err performance when returning nil.
Slows down Err when returning non-nil by about 3x,
but that's still almost 2x faster than before CL 653795
and the performance of this path is less important.
(A tight loop checking Err for doneness will be terminated
by the first Err call to return a non-nil result.)
goos: darwin
goarch: arm64
pkg: context
cpu: Apple M4 Pro
│ /tmp/bench.0 │ /tmp/bench.1 │
│ sec/op │ sec/op vs base │
ErrOK-14 1.806n ± 1% 1.774n ± 0% -1.77% (p=0.000 n=8)
ErrCanceled-14 1.821n ± 1% 7.525n ± 3% +313.23% (p=0.000 n=8)
geomean 1.813n 3.654n +101.47%
Fixes #75533
Fixes #75537
Change-Id: Iea22781a199ace7e7f70cf65168c36e090cd2e2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/705235
TryBot-Bypass: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
(cherry picked from commit 8ca209ec39
)
Reviewed-on: https://go-review.googlesource.com/c/go/+/705375
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This commit is contained in:
parent
0b53e410f8
commit
06993c7721
2 changed files with 22 additions and 0 deletions
|
@ -463,6 +463,8 @@ func (c *cancelCtx) Done() <-chan struct{} {
|
|||
func (c *cancelCtx) Err() error {
|
||||
// An atomic load is ~5x faster than a mutex, which can matter in tight loops.
|
||||
if err := c.err.Load(); err != nil {
|
||||
// Ensure the done channel has been closed before returning a non-nil error.
|
||||
<-c.Done()
|
||||
return err.(error)
|
||||
}
|
||||
return nil
|
||||
|
|
|
@ -1177,3 +1177,23 @@ func (c *customContext) Err() error {
|
|||
func (c *customContext) Value(key any) any {
|
||||
return c.parent.Value(key)
|
||||
}
|
||||
|
||||
// Issue #75533.
|
||||
func TestContextErrDoneRace(t *testing.T) {
|
||||
// 4 iterations reliably reproduced #75533.
|
||||
for range 10 {
|
||||
ctx, cancel := WithCancel(Background())
|
||||
donec := ctx.Done()
|
||||
go cancel()
|
||||
for ctx.Err() == nil {
|
||||
if runtime.GOARCH == "wasm" {
|
||||
runtime.Gosched() // need to explicitly yield
|
||||
}
|
||||
}
|
||||
select {
|
||||
case <-donec:
|
||||
default:
|
||||
t.Fatalf("ctx.Err is non-nil, but ctx.Done is not closed")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue