From d6c8bedc7b3d2de7714dca75bd05912b371538f1 Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Tue, 1 Apr 2025 15:43:22 -0700
Subject: [PATCH] runtime, testing/synctest: stop advancing time when main
goroutine exits
Once the goroutine started by synctest.Run exits, stop advancing
the fake clock in its bubble. This avoids confusing situations
where a bubble remains alive indefinitely while a background
goroutine reads from a time.Ticker or otherwise advances the clock.
For #67434
Change-Id: Id608ffe3c7d7b07747b56a21f365787fb9a057d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/662155
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
---
src/internal/synctest/synctest_test.go | 24 ++++++++++++++++++
src/runtime/synctest.go | 34 ++++++++++++++++++++------
src/testing/synctest/synctest.go | 5 +++-
3 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/src/internal/synctest/synctest_test.go b/src/internal/synctest/synctest_test.go
index 010679b070f..c0e126e3fcd 100644
--- a/src/internal/synctest/synctest_test.go
+++ b/src/internal/synctest/synctest_test.go
@@ -191,6 +191,18 @@ func TestTimeAfter(t *testing.T) {
})
}
+func TestTimerAfterBubbleExit(t *testing.T) {
+ run := false
+ synctest.Run(func() {
+ time.AfterFunc(1*time.Second, func() {
+ run = true
+ })
+ })
+ if run {
+ t.Errorf("timer ran before bubble exit")
+ }
+}
+
func TestTimerFromOutsideBubble(t *testing.T) {
tm := time.NewTimer(10 * time.Millisecond)
synctest.Run(func() {
@@ -308,6 +320,18 @@ func TestDeadlockChild(t *testing.T) {
})
}
+func TestDeadlockTicker(t *testing.T) {
+ defer wantPanic(t, "deadlock: all goroutines in bubble are blocked")
+ synctest.Run(func() {
+ go func() {
+ for range time.Tick(1 * time.Second) {
+ t.Errorf("ticker unexpectedly ran")
+ return
+ }
+ }()
+ })
+}
+
func TestCond(t *testing.T) {
synctest.Run(func() {
var mu sync.Mutex
diff --git a/src/runtime/synctest.go b/src/runtime/synctest.go
index b197758ad93..36d6fa67c71 100644
--- a/src/runtime/synctest.go
+++ b/src/runtime/synctest.go
@@ -5,6 +5,7 @@
package runtime
import (
+ "internal/runtime/sys"
"unsafe"
)
@@ -15,7 +16,9 @@ type synctestGroup struct {
now int64 // current fake time
root *g // caller of synctest.Run
waiter *g // caller of synctest.Wait
+ main *g // goroutine started by synctest.Run
waiting bool // true if a goroutine is calling synctest.Wait
+ done bool // true if main has exited
// The group is active (not blocked) so long as running > 0 || active > 0.
//
@@ -60,6 +63,9 @@ func (sg *synctestGroup) changegstatus(gp *g, oldval, newval uint32) {
case _Gdead:
isRunning = false
totalDelta--
+ if gp == sg.main {
+ sg.done = true
+ }
case _Gwaiting:
if gp.waitreason.isIdleInSynctest() {
isRunning = false
@@ -167,24 +173,32 @@ func synctestRun(f func()) {
if gp.syncGroup != nil {
panic("synctest.Run called from within a synctest bubble")
}
- gp.syncGroup = &synctestGroup{
+ sg := &synctestGroup{
total: 1,
running: 1,
root: gp,
}
const synctestBaseTime = 946684800000000000 // midnight UTC 2000-01-01
- gp.syncGroup.now = synctestBaseTime
- gp.syncGroup.timers.syncGroup = gp.syncGroup
- lockInit(&gp.syncGroup.mu, lockRankSynctest)
- lockInit(&gp.syncGroup.timers.mu, lockRankTimers)
+ sg.now = synctestBaseTime
+ sg.timers.syncGroup = sg
+ lockInit(&sg.mu, lockRankSynctest)
+ lockInit(&sg.timers.mu, lockRankTimers)
+
+ gp.syncGroup = sg
defer func() {
gp.syncGroup = nil
}()
- fv := *(**funcval)(unsafe.Pointer(&f))
- newproc(fv)
+ // This is newproc, but also records the new g in sg.main.
+ pc := sys.GetCallerPC()
+ systemstack(func() {
+ fv := *(**funcval)(unsafe.Pointer(&f))
+ sg.main = newproc1(fv, gp, pc, false, waitReasonZero)
+ pp := getg().m.p.ptr()
+ runqput(pp, sg.main, true)
+ wakep()
+ })
- sg := gp.syncGroup
lock(&sg.mu)
sg.active++
for {
@@ -209,6 +223,10 @@ func synctestRun(f func()) {
if next < sg.now {
throw("time went backwards")
}
+ if sg.done {
+ // Time stops once the bubble's main goroutine has exited.
+ break
+ }
sg.now = next
}
diff --git a/src/testing/synctest/synctest.go b/src/testing/synctest/synctest.go
index 90efc789de9..1b1aef2e79f 100644
--- a/src/testing/synctest/synctest.go
+++ b/src/testing/synctest/synctest.go
@@ -28,7 +28,10 @@ import (
// goroutines are blocked and return after the bubble's clock has
// advanced. See [Wait] for the specific definition of blocked.
//
-// If every goroutine is blocked and there are no timers scheduled,
+// Time stops advancing when f returns.
+//
+// If every goroutine is blocked and either
+// no timers are scheduled or f has returned,
// Run panics.
//
// Channels, time.Timers, and time.Tickers created within the bubble
--
GitLab