Skip to content

Commit

Permalink
patched-go: update src/runtime/time.go
Browse files Browse the repository at this point in the history
Cherry-pick [this commit](https://go-review.googlesource.com/c/go/+/617497)
to see if it fixes #132596.

Informs #132596

Epic: none
Release note: None
  • Loading branch information
rickystewart committed Oct 14, 2024
1 parent ce02890 commit 955b50d
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 14 deletions.
16 changes: 8 additions & 8 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,13 @@ load(
go_download_sdk(
name = "go_sdk",
sdks = {
"darwin_amd64": ("go1.23.2.darwin-amd64.tar.gz", "183cbd76aba98162e7bd0e69ce082ee613aaa643c36e7a5b364986f5a62e6b79"),
"darwin_arm64": ("go1.23.2.darwin-arm64.tar.gz", "3884b9302eeba170292286bf8c3fa9454635f175ae62a1c3f5a159bc1de5b14a"),
"linux_amd64": ("go1.23.2.linux-amd64.tar.gz", "9d5e626403c231dac511266e94a521e026ac5d81a8c161b1b7a1afa7de090b43"),
"linux_arm64": ("go1.23.2.linux-arm64.tar.gz", "3de3bdeef9e53eb442fafebf8bacaffeff223b9f5b24e29907c124c7da3036d9"),
"windows_amd64": ("go1.23.2.windows-amd64.tar.gz", "a4b821659c61473490d5842e571217ede2fae3356aad1c1d642457e27619b5bc"),
"darwin_amd64": ("go1.23.2.darwin-amd64.tar.gz", "116df0a832f7e9725f24d65a48247b5a6179e116072daee707bc53a7a7347773"),
"darwin_arm64": ("go1.23.2.darwin-arm64.tar.gz", "052a8fd9252624f63ed426c071a00f0d218bd81a02d3f9432f14cd098abbc80c"),
"linux_amd64": ("go1.23.2.linux-amd64.tar.gz", "2f31e9d56f0b96e2aac81225e41582863c99eb72e709490ad2ffc9233fa65c16"),
"linux_arm64": ("go1.23.2.linux-arm64.tar.gz", "3ad7b87b8980f8754146b4cfeb3d11c5ff2b4f28a522579928e9b80c7aa1bd5d"),
"windows_amd64": ("go1.23.2.windows-amd64.tar.gz", "c288879b4c9c249e10a455c053d8275d0ffa764a249c05ed9a493b0ca6922a02"),
},
urls = ["https://storage.googleapis.com/public-bazel-artifacts/go/20241008-180917/{}"],
urls = ["https://storage.googleapis.com/public-bazel-artifacts/go/20241014-193213/{}"],
version = "1.23.2",
)

Expand Down Expand Up @@ -654,8 +654,8 @@ go_download_sdk(
# able to provide additional diagnostic information such as the expected version of OpenSSL.
experiments = ["boringcrypto"],
sdks = {
"linux_amd64": ("go1.23.2fips.linux-amd64.tar.gz", "a5fd8eb584f4c134f3efbf05f9b2f994ce64ada633b8a26c5b51652b31e7a40f"),
"linux_amd64": ("go1.23.2fips.linux-amd64.tar.gz", "a1ef2818ff233c1f2adf39287b14775a3deb199480b1b15537a859633f861739"),
},
urls = ["https://storage.googleapis.com/public-bazel-artifacts/go/20241008-180917/{}"],
urls = ["https://storage.googleapis.com/public-bazel-artifacts/go/20241014-193213/{}"],
version = "1.23.2fips",
)
12 changes: 6 additions & 6 deletions build/bazelutil/distdir_files.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -1225,12 +1225,12 @@ DISTDIR_FILES = {
"https://storage.googleapis.com/public-bazel-artifacts/c-deps/20240419-195217/libproj_foreign.macos.20240419-195217.tar.gz": "4b4dadf30e225693723612ede7fc5138eb1ad1b863db744c52099535dbdc3c00",
"https://storage.googleapis.com/public-bazel-artifacts/c-deps/20240419-195217/libproj_foreign.macosarm.20240419-195217.tar.gz": "3e3220bd83009de29185772be26022ae219cb006eae1d8dba87292206ce9f4ea",
"https://storage.googleapis.com/public-bazel-artifacts/c-deps/20240419-195217/libproj_foreign.windows.20240419-195217.tar.gz": "8284b57f832ab3c5353860ad715e8844c93bf6822b01cb5108b5b494ea90a2dc",
"https://storage.googleapis.com/public-bazel-artifacts/go/20241008-180917/go1.23.2.darwin-amd64.tar.gz": "183cbd76aba98162e7bd0e69ce082ee613aaa643c36e7a5b364986f5a62e6b79",
"https://storage.googleapis.com/public-bazel-artifacts/go/20241008-180917/go1.23.2.darwin-arm64.tar.gz": "3884b9302eeba170292286bf8c3fa9454635f175ae62a1c3f5a159bc1de5b14a",
"https://storage.googleapis.com/public-bazel-artifacts/go/20241008-180917/go1.23.2.linux-amd64.tar.gz": "9d5e626403c231dac511266e94a521e026ac5d81a8c161b1b7a1afa7de090b43",
"https://storage.googleapis.com/public-bazel-artifacts/go/20241008-180917/go1.23.2.linux-arm64.tar.gz": "3de3bdeef9e53eb442fafebf8bacaffeff223b9f5b24e29907c124c7da3036d9",
"https://storage.googleapis.com/public-bazel-artifacts/go/20241008-180917/go1.23.2.windows-amd64.tar.gz": "a4b821659c61473490d5842e571217ede2fae3356aad1c1d642457e27619b5bc",
"https://storage.googleapis.com/public-bazel-artifacts/go/20241008-180917/go1.23.2fips.linux-amd64.tar.gz": "a5fd8eb584f4c134f3efbf05f9b2f994ce64ada633b8a26c5b51652b31e7a40f",
"https://storage.googleapis.com/public-bazel-artifacts/go/20241014-193213/go1.23.2.darwin-amd64.tar.gz": "116df0a832f7e9725f24d65a48247b5a6179e116072daee707bc53a7a7347773",
"https://storage.googleapis.com/public-bazel-artifacts/go/20241014-193213/go1.23.2.darwin-arm64.tar.gz": "052a8fd9252624f63ed426c071a00f0d218bd81a02d3f9432f14cd098abbc80c",
"https://storage.googleapis.com/public-bazel-artifacts/go/20241014-193213/go1.23.2.linux-amd64.tar.gz": "2f31e9d56f0b96e2aac81225e41582863c99eb72e709490ad2ffc9233fa65c16",
"https://storage.googleapis.com/public-bazel-artifacts/go/20241014-193213/go1.23.2.linux-arm64.tar.gz": "3ad7b87b8980f8754146b4cfeb3d11c5ff2b4f28a522579928e9b80c7aa1bd5d",
"https://storage.googleapis.com/public-bazel-artifacts/go/20241014-193213/go1.23.2.windows-amd64.tar.gz": "c288879b4c9c249e10a455c053d8275d0ffa764a249c05ed9a493b0ca6922a02",
"https://storage.googleapis.com/public-bazel-artifacts/go/20241014-193213/go1.23.2fips.linux-amd64.tar.gz": "a1ef2818ff233c1f2adf39287b14775a3deb199480b1b15537a859633f861739",
"https://storage.googleapis.com/public-bazel-artifacts/java/railroad/rr-1.63-java8.zip": "d2791cd7a44ea5be862f33f5a9b3d40aaad9858455828ebade7007ad7113fb41",
"https://storage.googleapis.com/public-bazel-artifacts/js/rules_jest-v0.18.4.tar.gz": "d3bb833f74b8ad054e6bff5e41606ff10a62880cc99e4d480f4bdfa70add1ba7",
"https://storage.googleapis.com/public-bazel-artifacts/js/rules_js-v1.42.3.tar.gz": "2cfb3875e1231cefd3fada6774f2c0c5a99db0070e0e48ea398acbff7c6c765b",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -544,3 +544,128 @@ index 43aba98dce..a076c93b8e 100644
{runtime.Sudog{}, 56, 88}, // sudog, but exported for testing
}

diff --git a/src/runtime/time.go b/src/runtime/time.go
index 6d47eba..3353502 100644
--- a/src/runtime/time.go
+++ b/src/runtime/time.go
@@ -1114,6 +1114,11 @@
// started to send the value. That lets them correctly return
// true meaning that no value was sent.
lock(&t.sendLock)
+
+ // We are committed to possibly sending a value based on seq,
+ // so no need to keep telling stop/modify that we are sending.
+ t.isSending.And(^isSendingClear)
+
if t.seq != seq {
f = func(any, uintptr, int64) {}
}
@@ -1122,9 +1127,6 @@
f(arg, seq, delay)

if !async && t.isChan {
- // We are no longer sending a value.
- t.isSending.And(^isSendingClear)
-
unlock(&t.sendLock)
}

diff --git a/src/runtime/time.go b/src/runtime/time.go
index 7abd15e..19b4ac9 100644
--- a/src/runtime/time.go
+++ b/src/runtime/time.go
@@ -33,6 +33,7 @@
// isSending is used to handle races between running a
// channel timer and stopping or resetting the timer.
// It is used only for channel timers (t.isChan == true).
+ // It is not used for tickers.
// The lowest zero bit is set when about to send a value on the channel,
// and cleared after sending the value.
// The stop/reset code uses this to detect whether it
@@ -467,7 +468,7 @@
// send from actually happening. That means
// that we should return true: the timer was
// stopped, even though t.when may be zero.
- if t.isSending.Load() > 0 {
+ if t.period == 0 && t.isSending.Load() > 0 {
pending = true
}
}
@@ -529,6 +530,7 @@
t.maybeRunAsync()
}
t.trace("modify")
+ oldPeriod := t.period
t.period = period
if f != nil {
t.f = f
@@ -570,7 +572,7 @@
// send from actually happening. That means
// that we should return true: the timer was
// stopped, even though t.when may be zero.
- if t.isSending.Load() > 0 {
+ if oldPeriod == 0 && t.isSending.Load() > 0 {
pending = true
}
}
@@ -1064,7 +1066,7 @@

async := debug.asynctimerchan.Load() != 0
var isSendingClear uint8
- if !async && t.isChan {
+ if !async && t.isChan && t.period == 0 {
// Tell Stop/Reset that we are sending a value.
// Set the lowest zero bit.
// We do this awkward step because atomic.Uint8
@@ -1115,9 +1117,12 @@
// true meaning that no value was sent.
lock(&t.sendLock)

- // We are committed to possibly sending a value based on seq,
- // so no need to keep telling stop/modify that we are sending.
- t.isSending.And(^isSendingClear)
+ if t.period == 0 {
+ // We are committed to possibly sending a value
+ // based on seq, so no need to keep telling
+ // stop/modify that we are sending.
+ t.isSending.And(^isSendingClear)
+ }

if t.seq != seq {
f = func(any, uintptr, int64) {}
diff --git a/src/time/sleep_test.go b/src/time/sleep_test.go
index 5357ed2..520ff95 100644
--- a/src/time/sleep_test.go
+++ b/src/time/sleep_test.go
@@ -847,6 +847,31 @@
wg.Wait()
}

+// Test having a large number of goroutines wake up a timer simultaneously.
+// This used to trigger a crash when run under x/tools/cmd/stress.
+func TestMultiWakeup(t *testing.T) {
+ if testing.Short() {
+ t.Skip("-short")
+ }
+
+ goroutines := runtime.GOMAXPROCS(0)
+ timer := NewTicker(Microsecond)
+ var wg sync.WaitGroup
+ wg.Add(goroutines)
+ for range goroutines {
+ go func() {
+ defer wg.Done()
+ for range 100000 {
+ select {
+ case <-timer.C:
+ case <-After(Millisecond):
+ }
+ }
+ }()
+ }
+ wg.Wait()
+}
+
// Benchmark timer latency when the thread that creates the timer is busy with
// other work and the timers must be serviced by other threads.
// https://golang.org/issue/38860

0 comments on commit 955b50d

Please sign in to comment.