diff options
author | Ian Lance Taylor <iant@golang.org> | 2021-07-21 19:57:56 -0700 |
---|---|---|
committer | Ian Lance Taylor <iant@golang.org> | 2021-07-22 21:53:16 +0000 |
commit | ed8cbbc3ae96aef98d8f9e9e7003a99ed74992b5 (patch) | |
tree | 45f7d4b0d7ab98665d3d5caa64ee461433fbd904 | |
parent | bc51e930274a5d5835ac8797978afc0864c9e30c (diff) | |
download | go-ed8cbbc3ae96aef98d8f9e9e7003a99ed74992b5.tar.gz go-ed8cbbc3ae96aef98d8f9e9e7003a99ed74992b5.zip |
[release-branch.go1.16] runtime: don't clear timerModifiedEarliest if adjustTimers is 0
This avoids a race when a new timerModifiedEarlier timer is created by
a different goroutine.
For #47329
Fixes #47332
Change-Id: I6f6c87b4a9b5491b201c725c10bc98e23e0ed9d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/336432
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit 798ec73519a7226d6d436e42498a54aed23b8468)
Reviewed-on: https://go-review.googlesource.com/c/go/+/336689
-rw-r--r-- | src/runtime/runtime2.go | 2 | ||||
-rw-r--r-- | src/runtime/time.go | 5 | ||||
-rw-r--r-- | src/time/sleep_test.go | 34 |
3 files changed, 35 insertions, 6 deletions
diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 9a032d8658..dd375da193 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -653,7 +653,7 @@ type p struct { // timerModifiedEarlier status. Because the timer may have been // modified again, there need not be any timer with this value. // This is updated using atomic functions. - // This is 0 if the value is unknown. + // This is 0 if there are no timerModifiedEarlier timers. timerModifiedEarliest uint64 // Per-P GC state diff --git a/src/runtime/time.go b/src/runtime/time.go index dee6a674e4..7b84d2af57 100644 --- a/src/runtime/time.go +++ b/src/runtime/time.go @@ -668,11 +668,6 @@ func adjusttimers(pp *p, now int64) { if verifyTimers { verifyTimerHeap(pp) } - // There are no timers to adjust, so it is safe to clear - // timerModifiedEarliest. Do so in case it is stale. - // Everything will work if we don't do this, - // but clearing here may save future calls to adjusttimers. - atomic.Store64(&pp.timerModifiedEarliest, 0) return } diff --git a/src/time/sleep_test.go b/src/time/sleep_test.go index 6ee0631a85..e0172bf5e0 100644 --- a/src/time/sleep_test.go +++ b/src/time/sleep_test.go @@ -527,6 +527,40 @@ func TestZeroTimer(t *testing.T) { } } +// Test that rapidly moving a timer earlier doesn't cause it to get dropped. +// Issue 47329. +func TestTimerModifiedEarlier(t *testing.T) { + past := Until(Unix(0, 0)) + count := 1000 + fail := 0 + for i := 0; i < count; i++ { + timer := NewTimer(Hour) + for j := 0; j < 10; j++ { + if !timer.Stop() { + <-timer.C + } + timer.Reset(past) + } + + deadline := NewTimer(10 * Second) + defer deadline.Stop() + now := Now() + select { + case <-timer.C: + if since := Since(now); since > 8*Second { + t.Errorf("timer took too long (%v)", since) + fail++ + } + case <-deadline.C: + t.Error("deadline expired") + } + } + + if fail > 0 { + t.Errorf("%d failures", fail) + } +} + // 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 |