Skip to content

Commit 4ce116a

Browse files
neildgopherbot
authored andcommitted
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 <[email protected]> Reviewed-by: Michael Pratt <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent e6de1b2 commit 4ce116a

File tree

2 files changed

+46
-5
lines changed

2 files changed

+46
-5
lines changed

src/internal/synctest/synctest_test.go

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func TestMallocs(t *testing.T) {
105105
}
106106
}
107107

108-
func TestTimer(t *testing.T) {
108+
func TestTimerReadBeforeDeadline(t *testing.T) {
109109
synctest.Run(func() {
110110
start := time.Now()
111111
tm := time.NewTimer(5 * time.Second)
@@ -116,6 +116,41 @@ func TestTimer(t *testing.T) {
116116
})
117117
}
118118

119+
func TestTimerReadAfterDeadline(t *testing.T) {
120+
synctest.Run(func() {
121+
delay := 1 * time.Second
122+
want := time.Now().Add(delay)
123+
tm := time.NewTimer(delay)
124+
time.Sleep(2 * delay)
125+
got := <-tm.C
126+
if got != want {
127+
t.Errorf("<-tm.C = %v, want %v", got, want)
128+
}
129+
})
130+
}
131+
132+
func TestTimerReset(t *testing.T) {
133+
synctest.Run(func() {
134+
start := time.Now()
135+
tm := time.NewTimer(1 * time.Second)
136+
if got, want := <-tm.C, start.Add(1*time.Second); got != want {
137+
t.Errorf("first sleep: <-tm.C = %v, want %v", got, want)
138+
}
139+
140+
tm.Reset(2 * time.Second)
141+
if got, want := <-tm.C, start.Add((1+2)*time.Second); got != want {
142+
t.Errorf("second sleep: <-tm.C = %v, want %v", got, want)
143+
}
144+
145+
tm.Reset(3 * time.Second)
146+
time.Sleep(1 * time.Second)
147+
tm.Reset(3 * time.Second)
148+
if got, want := <-tm.C, start.Add((1+2+4)*time.Second); got != want {
149+
t.Errorf("third sleep: <-tm.C = %v, want %v", got, want)
150+
}
151+
})
152+
}
153+
119154
func TestTimeAfter(t *testing.T) {
120155
synctest.Run(func() {
121156
i := 0
@@ -138,9 +173,11 @@ func TestTimeAfter(t *testing.T) {
138173
func TestTimerFromOutsideBubble(t *testing.T) {
139174
tm := time.NewTimer(10 * time.Millisecond)
140175
synctest.Run(func() {
141-
defer wantPanic(t, "timer moved between synctest groups")
142176
<-tm.C
143177
})
178+
if tm.Stop() {
179+
t.Errorf("synctest.Run unexpectedly returned before timer fired")
180+
}
144181
}
145182

146183
func TestChannelFromOutsideBubble(t *testing.T) {

src/runtime/time.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1370,15 +1370,19 @@ func badTimer() {
13701370
// to send a value to its associated channel. If so, it does.
13711371
// The timer must not be locked.
13721372
func (t *timer) maybeRunChan() {
1373-
if sg := getg().syncGroup; sg != nil || t.isFake {
1373+
if t.isFake {
13741374
t.lock()
13751375
var timerGroup *synctestGroup
13761376
if t.ts != nil {
13771377
timerGroup = t.ts.syncGroup
13781378
}
13791379
t.unlock()
1380-
if sg == nil || !t.isFake || sg != timerGroup {
1381-
panic(plainError("timer moved between synctest groups"))
1380+
sg := getg().syncGroup
1381+
if sg == nil {
1382+
panic(plainError("synctest timer accessed from outside bubble"))
1383+
}
1384+
if timerGroup != nil && sg != timerGroup {
1385+
panic(plainError("timer moved between synctest bubbles"))
13821386
}
13831387
// No need to do anything here.
13841388
// synctest.Run will run the timer when it advances its fake clock.

0 commit comments

Comments
 (0)