aboutsummaryrefslogtreecommitdiff
path: root/src/time
diff options
context:
space:
mode:
authorRuss Cox <rsc@golang.org>2024-03-13 19:48:58 -0400
committerGopher Robot <gobot@golang.org>2024-03-14 14:01:19 +0000
commit2965dc989530e1f52d80408503be24ad2582871b (patch)
treecf989cdf88c029f3dc4f62a76a01a5d4eac599d4 /src/time
parent9159d71a37a7d2adfc172f6851876fc36d1f0f38 (diff)
downloadgo-2965dc989530e1f52d80408503be24ad2582871b.tar.gz
go-2965dc989530e1f52d80408503be24ad2582871b.zip
runtime: fix lost sleep causing TestZeroTimer flakes
Classic operating system kernel mistake: if you start using per-CPU data without disabling interrupts on the CPU, and then an interrupt reschedules the process onto a different CPU, now you're using the wrong CPU's per-CPU data. The same thing happens in Go if you use per-M or per-P data structures while not holding a lock nor using acquirem. In the original timer.modify before CL 564977, I had been very careful about this during the "unlock t; lock ts" dance, only calling releasem after ts was locked. That made sure we used the right ts. The refactoring of that code into its own helper function in CL 564977 missed that nuance. The code ts := &getg().m.p.p.ptr().timers ts.lock() was now executing without holding any locks nor acquirem. If the goroutine changed its M or P between deciding which ts to use and actually locking that ts, the code would proceed to add the timer t to some other P's timers. If the P was idle by then, the scheduler could have already checked it for timers and not notice the newly added timer when deciding when the next timer should trigger. The solution is to do what the old code correctly did, namely acquirem before deciding which ts to use, rather than assume getg().m.p won't change before ts.lock can complete. This CL does that. Before CL 564977, stress ./time.test -test.run='ZeroTimer/impl=(func|cache)' -test.timeout=3m -test.count=20 ran without failure for over an hour on my laptop. Starting in CL 564977, it consistently failed within a few minutes. After this CL, it now runs without failure for over an hour again. Fixes #66006. Change-Id: Ib9e7ccaa0f22a326ce3fdef2b9a92f7f0bdafcbf Reviewed-on: https://go-review.googlesource.com/c/go/+/571196 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Diffstat (limited to 'src/time')
-rw-r--r--src/time/sleep_test.go7
1 files changed, 7 insertions, 0 deletions
diff --git a/src/time/sleep_test.go b/src/time/sleep_test.go
index 565af16d4d..8c28b1e4a9 100644
--- a/src/time/sleep_test.go
+++ b/src/time/sleep_test.go
@@ -656,6 +656,13 @@ func TestZeroTimer(t *testing.T) {
t.Run("impl=func", func(t *testing.T) {
testZeroTimer(t, newTimerFunc)
})
+ t.Run("impl=cache", func(t *testing.T) {
+ timer := newTimerFunc(Hour)
+ testZeroTimer(t, func(d Duration) *Timer {
+ timer.Reset(d)
+ return timer
+ })
+ })
}
func testZeroTimer(t *testing.T, newTimer func(Duration) *Timer) {