Skip to content

Commit 0da04a6

Browse files
prattmictoothrot
authored andcommitted
[release-branch.go1.16] runtime, time: disable preemption in addtimer
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 #44869 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]> (cherry picked from commit aa26687) Reviewed-on: https://go-review.googlesource.com/c/go/+/300611
1 parent 3979fb9 commit 0da04a6

File tree

2 files changed

+21
-0
lines changed

2 files changed

+21
-0
lines changed

src/runtime/time.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,13 +263,18 @@ func addtimer(t *timer) {
263263

264264
when := t.when
265265

266+
// Disable preemption while using pp to avoid changing another P's heap.
267+
mp := acquirem()
268+
266269
pp := getg().m.p.ptr()
267270
lock(&pp.timersLock)
268271
cleantimers(pp)
269272
doaddtimer(pp, t)
270273
unlock(&pp.timersLock)
271274

272275
wakeNetPoller(when)
276+
277+
releasem(mp)
273278
}
274279

275280
// doaddtimer adds t to the current P's heap.

src/time/sleep_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,22 @@ func TestZeroTimerStopPanics(t *testing.T) {
511511
tr.Stop()
512512
}
513513

514+
// Test that zero duration timers aren't missed by the scheduler. Regression test for issue 44868.
515+
func TestZeroTimer(t *testing.T) {
516+
if testing.Short() {
517+
t.Skip("-short")
518+
}
519+
520+
for i := 0; i < 1000000; i++ {
521+
s := Now()
522+
ti := NewTimer(0)
523+
<-ti.C
524+
if diff := Since(s); diff > 2*Second {
525+
t.Errorf("Expected time to get value from Timer channel in less than 2 sec, took %v", diff)
526+
}
527+
}
528+
}
529+
514530
// Benchmark timer latency when the thread that creates the timer is busy with
515531
// other work and the timers must be serviced by other threads.
516532
// https://golang.org/issue/38860

0 commit comments

Comments
 (0)