Skip to content

Commit

Permalink
[release-branch.go1.15] runtime, time: disable preemption in addtimer
Browse files Browse the repository at this point in the history
The timerpMask optimization updates a mask of Ps (potentially)
containing timers in pidleget / pidleput. For correctness, it depends on
the assumption that new timers can only be added to a P's own heap.

addtimer violates this assumption if it is preempted after computing pp.
That G may then run on a different P, but adding a timer to the original
P's heap.

Avoid this by disabling preemption while pp is in use.

Other uses of doaddtimer should be OK:

* moveTimers: always moves to the current P's heap
* modtimer, cleantimers, addAdjustedTimers, runtimer: does not add net
  new timers to the heap while locked

For #44868
Fixes #45731

Change-Id: I4a5d080865e854931d0a3a09a51ca36879101d72
Reviewed-on: https://go-review.googlesource.com/c/go/+/300610
Trust: Michael Pratt <[email protected]>
Run-TryBot: Michael Pratt <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/313129
Trust: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
  • Loading branch information
prattmic authored and ianlancetaylor committed Apr 29, 2021
1 parent 5aed4ce commit 72ccabc
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/runtime/time.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,13 +254,18 @@ func addtimer(t *timer) {

when := t.when

// Disable preemption while using pp to avoid changing another P's heap.
mp := acquirem()

pp := getg().m.p.ptr()
lock(&pp.timersLock)
cleantimers(pp)
doaddtimer(pp, t)
unlock(&pp.timersLock)

wakeNetPoller(when)

releasem(mp)
}

// doaddtimer adds t to the current P's heap.
Expand Down
16 changes: 16 additions & 0 deletions src/time/sleep_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -501,3 +501,19 @@ func TestZeroTimerStopPanics(t *testing.T) {
var tr Timer
tr.Stop()
}

// Test that zero duration timers aren't missed by the scheduler. Regression test for issue 44868.
func TestZeroTimer(t *testing.T) {
if testing.Short() {
t.Skip("-short")
}

for i := 0; i < 1000000; i++ {
s := Now()
ti := NewTimer(0)
<-ti.C
if diff := Since(s); diff > 2*Second {
t.Errorf("Expected time to get value from Timer channel in less than 2 sec, took %v", diff)
}
}
}

0 comments on commit 72ccabc

Please sign in to comment.