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>

diff --git a/src/internal/synctest/synctest_test.go b/src/internal/synctest/synctest_test.go
index 2c4ac0ff64b6b0c24e096f4fd96382304e12b76f..7d1e04e2ba34e21d23a45a0ad5866ec62e67de30 100644 (file)
--- 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 7c6d7988726f069b1601b470fafd32c71443335c..c22d39c0894e35c0d4c8c383d38371cb28da71c0 100644 (file)
--- 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.