Skip to content

Commit 385dc33

Browse files
nicholashusingopherbot
authored andcommitted
runtime: prevent time.Timer.Reset(0) from deadlocking testing/synctest tests
In Go 1.23+, timer channels behave synchronously. When we have a timer channel (i.e. !async && t.isChan) we would lock the runtime.timer.sendLock mutex at the beginning of runtime.timer.modify()'s execution. Calling time.Timer.Reset(0) within a testing/synctest test, unfortunately, causes it to hang indefinitely. This is because the runtime.timer.sendLock mutex ends up being locked twice before it could be unlocked: - When calling time.Timer.Reset(), runtime.timer.modify() would lock the mutex per usual. - Due to the 0 argument, runtime.timer.modify() would also try to execute the bubbled timer immediately rather than adding them to a heap. However, in doing so, it uses runtime.timer.unlockAndRun(), which also locks the same mutex. This CL solves this issue by making sure that a locked runtime.timer.sendLock mutex is unlocked first, whenever we try to execute bubbled timer immediately in the stack. Fixes #76052 Change-Id: I66429b9bf6971400de95dcf2d5dc9670c3135492 Reviewed-on: https://go-review.googlesource.com/c/go/+/716883 Reviewed-by: Damien Neil <dneil@google.com> Auto-Submit: Nicholas Husin <nsh@golang.org> Reviewed-by: Nicholas Husin <husin@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent 99b724f commit 385dc33

File tree

2 files changed

+29
-0
lines changed

2 files changed

+29
-0
lines changed

src/internal/synctest/synctest_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package synctest_test
66

77
import (
8+
"context"
89
"fmt"
910
"internal/synctest"
1011
"internal/testenv"
@@ -329,6 +330,31 @@ func TestAfterFuncRunsImmediately(t *testing.T) {
329330
})
330331
}
331332

333+
// TestTimerResetZeroDoNotHang verifies that using timer.Reset(0) does not
334+
// cause the test to hang indefinitely. See https://go.dev/issue/76052.
335+
func TestTimerResetZeroDoNotHang(t *testing.T) {
336+
synctest.Run(func() {
337+
timer := time.NewTimer(0)
338+
ctx, cancel := context.WithCancel(context.Background())
339+
340+
go func() {
341+
for {
342+
select {
343+
case <-ctx.Done():
344+
return
345+
case <-timer.C:
346+
}
347+
}
348+
}()
349+
350+
synctest.Wait()
351+
timer.Reset(0)
352+
synctest.Wait()
353+
cancel()
354+
synctest.Wait()
355+
})
356+
}
357+
332358
func TestChannelFromOutsideBubble(t *testing.T) {
333359
choutside := make(chan struct{})
334360
for _, test := range []struct {

src/runtime/time.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,9 @@ func (t *timer) modify(when, period int64, f func(arg any, seq uintptr, delay in
636636
}
637637
if t.state&timerHeaped == 0 && when <= bubble.now {
638638
systemstack(func() {
639+
if !async && t.isChan {
640+
unlock(&t.sendLock)
641+
}
639642
t.unlockAndRun(bubble.now, bubble)
640643
})
641644
return pending

0 commit comments

Comments
 (0)