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 2c4ac0ff64..7d1e04e2ba 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 7c6d798872..c22d39c089 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.
-- 
2.48.1