[go] context: don't return a non-nil from Err before Done is closed
3 views
Skip to first unread message
Damien Neil (Gerrit)
Sep 18, 2025, 2:20:25 PM (13 days ago) Sep 18
to goph...@pubsubhelper.golang.org, Nicholas Husin, Go LUCI, golang-co...@googlegroups.com
Attention needed from Nicholas Husin
New activity on the change![]( "Open in Gerrit")
Related details
Attention is currently required from:
- Nicholas Husin
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicholas Husin (Gerrit)
Sep 18, 2025, 2:22:59 PM (13 days ago) Sep 18
to Damien Neil, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
Attention needed from Damien Neil
Nicholas Husin voted![]( "Open in Gerrit")
Attention is currently required from:
- Damien Neil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicholas Husin (Gerrit)
Sep 18, 2025, 2:23:10 PM (13 days ago) Sep 18
to Damien Neil, goph...@pubsubhelper.golang.org, Nicholas Husin, Go LUCI, golang-co...@googlegroups.com
Attention needed from Damien Neil
Nicholas Husin voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Damien Neil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Sep 18, 2025, 4:45:36 PM (13 days ago) Sep 18
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Damien Neil and Nicholas Husin
Damien Neil uploaded new patchset![]( "Open in Gerrit")
Damien Neil uploaded patch set #2 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI
Related details
Attention is currently required from:
- Damien Neil
- Nicholas Husin
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Sep 18, 2025, 5:05:41 PM (13 days ago) Sep 18
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Damien Neil and Nicholas Husin
Damien Neil uploaded new patchset![]( "Open in Gerrit")
Damien Neil uploaded patch set #3 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI
Related details
Attention is currently required from:
- Damien Neil
- Nicholas Husin
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Sep 18, 2025, 5:33:49 PM (13 days ago) Sep 18
to goph...@pubsubhelper.golang.org, Go LUCI, Nicholas Husin, Nicholas Husin, golang-co...@googlegroups.com
Attention needed from Nicholas Husin
Damien Neil voted and added 1 comment![]( "Open in Gerrit")
Votes added by Damien Neil
| Auto-Submit | +1 |
| TryBot-Bypass | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Nicholas Husin
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gopher Robot (Gerrit)
Sep 18, 2025, 5:34:18 PM (13 days ago) Sep 18
to Damien Neil, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Nicholas Husin, Nicholas Husin, golang-co...@googlegroups.com
Gopher Robot submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/context/x_test.go
Insertions: 3, Deletions: 0.
@@ -1186,6 +1186,9 @@
donec := ctx.Done()
go cancel()
for ctx.Err() == nil {
+ if runtime.GOARCH == "wasm" {
+ runtime.Gosched() // need to explicitly yield
+ }
}
select {
case <-donec:
```
Change information
Commit message:
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
Change-Id: Iea22781a199ace7e7f70cf65168c36e090cd2e2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/705235
TryBot-Bypass: Damien Neil <dn...@google.com>
Reviewed-by: Nicholas Husin <hu...@google.com>
Reviewed-by: Nicholas Husin <n...@golang.org>
Auto-Submit: Damien Neil <dn...@google.com>
Files:
- M src/context/context.go
- M src/context/x_test.go
Change size: S
Delta: 2 files changed, 22 insertions(+), 0 deletions(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +2 by Nicholas Husin, +1 by Nicholas Husin
TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI, TryBot-Bypass+1 by Damien Neil
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Sep 18, 2025, 5:37:29 PM (13 days ago) Sep 18
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Damien Neil has uploaded the change for review![]( "Open in Gerrit")
Commit message
[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 <dn...@google.com>
Reviewed-by: Nicholas Husin <hu...@google.com>
Reviewed-by: Nicholas Husin <n...@golang.org>
Auto-Submit: Damien Neil <dn...@google.com>
(cherry picked from commit 8ca209ec3962874ad1c15c22c86293edf428c284)
Change diff
diff --git a/src/context/context.go b/src/context/context.go
index 4f150f6..24bb18a 100644
--- a/src/context/context.go
+++ b/src/context/context.go
@@ -463,6 +463,8 @@
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
diff --git a/src/context/x_test.go b/src/context/x_test.go
index 937cab1..0cf1968 100644
--- a/src/context/x_test.go
+++ b/src/context/x_test.go
@@ -1177,3 +1177,23 @@
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")
+ }
+ }
+}
Change information
Files:
- M src/context/context.go
- M src/context/x_test.go
Change size: S
Delta: 2 files changed, 22 insertions(+), 0 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Carlos Amedee (Gerrit)
11:15 AM (11 hours ago) 11:15 AM
to Damien Neil, goph...@pubsubhelper.golang.org, Nicholas Husin, Cherry Mui, Sameer Ajmani, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Damien Neil, Nicholas Husin and Sameer Ajmani
Carlos Amedee voted Commit-Queue+1![]( "Open in Gerrit")
Attention is currently required from:
- Damien Neil
- Nicholas Husin
- Sameer Ajmani
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicholas Husin (Gerrit)
11:22 AM (11 hours ago) 11:22 AM
to Damien Neil, goph...@pubsubhelper.golang.org, Go LUCI, Carlos Amedee, Cherry Mui, Sameer Ajmani, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Damien Neil and Sameer Ajmani
Nicholas Husin voted Code-Review+2![]( "Open in Gerrit")
Attention is currently required from:
- Damien Neil
- Sameer Ajmani
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicholas Husin (Gerrit)
11:29 AM (11 hours ago) 11:29 AM
to Damien Neil, goph...@pubsubhelper.golang.org, Nicholas Husin, Go LUCI, Carlos Amedee, Cherry Mui, Sameer Ajmani, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Damien Neil and Sameer Ajmani
Nicholas Husin voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Damien Neil
- Sameer Ajmani
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cherry Mui (Gerrit)
11:32 AM (11 hours ago) 11:32 AM
to Damien Neil, goph...@pubsubhelper.golang.org, Nicholas Husin, Nicholas Husin, Go LUCI, Carlos Amedee, Sameer Ajmani, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Damien Neil and Sameer Ajmani
Cherry Mui voted Code-Review+2![]( "Open in Gerrit")
| Code-Review | +2 |
Related details
Attention is currently required from:
- Damien Neil
- Sameer Ajmani
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Carlos Amedee (Gerrit)
2:42 PM (8 hours ago) 2:42 PM
to Damien Neil, goph...@pubsubhelper.golang.org, Go LUCI, Cherry Mui, Nicholas Husin, Nicholas Husin, Sameer Ajmani, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Damien Neil and Sameer Ajmani
Carlos Amedee voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Damien Neil
- Sameer Ajmani
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Carlos Amedee (Gerrit)
3:06 PM (7 hours ago) 3:06 PM
to Damien Neil, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Cherry Mui, Nicholas Husin, Nicholas Husin, Sameer Ajmani, Gopher Robot, golang-co...@googlegroups.com
Carlos Amedee submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Reviewed-on: https://go-review.googlesource.com/c/go/+/705375
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cher...@google.com>
Files:
- M src/context/context.go
- M src/context/x_test.go
Change size: S
Delta: 2 files changed, 22 insertions(+), 0 deletions(-)
Branch: refs/heads/release-branch.go1.25
Submit Requirements:
Code-Review: +2 by Cherry Mui, +1 by Nicholas Husin, +2 by Nicholas Husin
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages