Skip to content

Commit

Permalink
[release-branch.go1.16] runtime: don't clear timerModifiedEarliest if…
Browse files Browse the repository at this point in the history
… 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 <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
(cherry picked from commit 798ec73)
Reviewed-on: https://go-review.googlesource.com/c/go/+/336689
  • Loading branch information
ianlancetaylor committed Jul 22, 2021
1 parent bc51e93 commit ed8cbbc
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/runtime/runtime2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions src/runtime/time.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
34 changes: 34 additions & 0 deletions src/time/sleep_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ed8cbbc

Please sign in to comment.