From 4ce116a884bd55d7046dbc999f329fa417414c00 Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Tue, 10 Dec 2024 09:49:45 -0800
Subject: [PATCH] runtime: avoid panic in expired synctest timer chan read
When reading from time.Timer.C for an expired timer using
a fake clock (in a synctest bubble), the timer will not
be in a heap. Avoid a spurious panic claiming the timer
moved between synctest bubbles.
Drop the panic when a bubbled goroutine reads from a
non-bubbled timer channel: We allow bubbled goroutines
to access non-bubbled channels in general.
Fixes #70741
Change-Id: I27005e46f4d0067cc6846d234d22766d2e05d163
Reviewed-on: https://go-review.googlesource.com/c/go/+/634955
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---
src/internal/synctest/synctest_test.go | 41 ++++++++++++++++++++++++--
src/runtime/time.go | 10 +++++--
2 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/src/internal/synctest/synctest_test.go b/src/internal/synctest/synctest_test.go
index 2c4ac0ff64b..7d1e04e2ba3 100644
--- a/src/internal/synctest/synctest_test.go
+++ b/src/internal/synctest/synctest_test.go
@@ -105,7 +105,7 @@ func TestMallocs(t *testing.T) {
}
}
-func TestTimer(t *testing.T) {
+func TestTimerReadBeforeDeadline(t *testing.T) {
synctest.Run(func() {
start := time.Now()
tm := time.NewTimer(5 * time.Second)
@@ -116,6 +116,41 @@ func TestTimer(t *testing.T) {
})
}
+func TestTimerReadAfterDeadline(t *testing.T) {
+ synctest.Run(func() {
+ delay := 1 * time.Second
+ want := time.Now().Add(delay)
+ tm := time.NewTimer(delay)
+ time.Sleep(2 * delay)
+ got := <-tm.C
+ if got != want {
+ t.Errorf("<-tm.C = %v, want %v", got, want)
+ }
+ })
+}
+
+func TestTimerReset(t *testing.T) {
+ synctest.Run(func() {
+ start := time.Now()
+ tm := time.NewTimer(1 * time.Second)
+ if got, want := <-tm.C, start.Add(1*time.Second); got != want {
+ t.Errorf("first sleep: <-tm.C = %v, want %v", got, want)
+ }
+
+ tm.Reset(2 * time.Second)
+ if got, want := <-tm.C, start.Add((1+2)*time.Second); got != want {
+ t.Errorf("second sleep: <-tm.C = %v, want %v", got, want)
+ }
+
+ tm.Reset(3 * time.Second)
+ time.Sleep(1 * time.Second)
+ tm.Reset(3 * time.Second)
+ if got, want := <-tm.C, start.Add((1+2+4)*time.Second); got != want {
+ t.Errorf("third sleep: <-tm.C = %v, want %v", got, want)
+ }
+ })
+}
+
func TestTimeAfter(t *testing.T) {
synctest.Run(func() {
i := 0
@@ -138,9 +173,11 @@ func TestTimeAfter(t *testing.T) {
func TestTimerFromOutsideBubble(t *testing.T) {
tm := time.NewTimer(10 * time.Millisecond)
synctest.Run(func() {
- defer wantPanic(t, "timer moved between synctest groups")
<-tm.C
})
+ if tm.Stop() {
+ t.Errorf("synctest.Run unexpectedly returned before timer fired")
+ }
}
func TestChannelFromOutsideBubble(t *testing.T) {
diff --git a/src/runtime/time.go b/src/runtime/time.go
index 7c6d7988726..c22d39c0894 100644
--- a/src/runtime/time.go
+++ b/src/runtime/time.go
@@ -1370,15 +1370,19 @@ func badTimer() {
// to send a value to its associated channel. If so, it does.
// The timer must not be locked.
func (t *timer) maybeRunChan() {
- if sg := getg().syncGroup; sg != nil || t.isFake {
+ if t.isFake {
t.lock()
var timerGroup *synctestGroup
if t.ts != nil {
timerGroup = t.ts.syncGroup
}
t.unlock()
- if sg == nil || !t.isFake || sg != timerGroup {
- panic(plainError("timer moved between synctest groups"))
+ sg := getg().syncGroup
+ if sg == nil {
+ panic(plainError("synctest timer accessed from outside bubble"))
+ }
+ if timerGroup != nil && sg != timerGroup {
+ panic(plainError("timer moved between synctest bubbles"))
}
// No need to do anything here.
// synctest.Run will run the timer when it advances its fake clock.
--
GitLab