aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIan Lance Taylor <iant@golang.org>2021-07-21 19:57:56 -0700
committerIan Lance Taylor <iant@golang.org>2021-07-22 21:53:16 +0000
commited8cbbc3ae96aef98d8f9e9e7003a99ed74992b5 (patch)
tree45f7d4b0d7ab98665d3d5caa64ee461433fbd904
parentbc51e930274a5d5835ac8797978afc0864c9e30c (diff)
downloadgo-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.go2
-rw-r--r--src/runtime/time.go5
-rw-r--r--src/time/sleep_test.go34
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