Skip to content

Commit 1471978

Browse files
committed
iter: propagate panics from the iterator passed to Pull
This change propagates panics from the iterator passed to Pull through next and stop. Once the panic occurs, next and stop become no-ops (the iterator is invalidated). For #67712. Change-Id: I05e45601d4d10acdf51b53e3164bd891c1b324ac Reviewed-on: https://go-review.googlesource.com/c/go/+/589136 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Austin Clements <austin@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent d5e5b14 commit 1471978

File tree

2 files changed

+132
-13
lines changed

2 files changed

+132
-13
lines changed

Diff for: src/iter/iter.go

+53-13
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,12 @@ func coroswitch(*coro)
5050
// simultaneously.
5151
func Pull[V any](seq Seq[V]) (next func() (V, bool), stop func()) {
5252
var (
53-
v V
54-
ok bool
55-
done bool
56-
yieldNext bool
57-
racer int
53+
v V
54+
ok bool
55+
done bool
56+
yieldNext bool
57+
racer int
58+
panicValue any
5859
)
5960
c := newcoro(func(c *coro) {
6061
race.Acquire(unsafe.Pointer(&racer))
@@ -72,14 +73,22 @@ func Pull[V any](seq Seq[V]) (next func() (V, bool), stop func()) {
7273
race.Acquire(unsafe.Pointer(&racer))
7374
return !done
7475
}
76+
// Recover and propagate panics from seq.
77+
defer func() {
78+
if p := recover(); p != nil {
79+
done = true // Invalidate iterator.
80+
panicValue = p
81+
}
82+
race.Release(unsafe.Pointer(&racer))
83+
}()
7584
seq(yield)
7685
var v0 V
7786
v, ok = v0, false
7887
done = true
79-
race.Release(unsafe.Pointer(&racer))
8088
})
8189
next = func() (v1 V, ok1 bool) {
8290
race.Write(unsafe.Pointer(&racer)) // detect races
91+
8392
if done {
8493
return
8594
}
@@ -90,15 +99,26 @@ func Pull[V any](seq Seq[V]) (next func() (V, bool), stop func()) {
9099
race.Release(unsafe.Pointer(&racer))
91100
coroswitch(c)
92101
race.Acquire(unsafe.Pointer(&racer))
102+
103+
// Propagate panics from seq.
104+
if panicValue != nil {
105+
panic(panicValue)
106+
}
93107
return v, ok
94108
}
95109
stop = func() {
96110
race.Write(unsafe.Pointer(&racer)) // detect races
111+
97112
if !done {
98113
done = true
99114
race.Release(unsafe.Pointer(&racer))
100115
coroswitch(c)
101116
race.Acquire(unsafe.Pointer(&racer))
117+
118+
// Propagate panics from seq.
119+
if panicValue != nil {
120+
panic(panicValue)
121+
}
102122
}
103123
}
104124
return next, stop
@@ -125,12 +145,13 @@ func Pull[V any](seq Seq[V]) (next func() (V, bool), stop func()) {
125145
// simultaneously.
126146
func Pull2[K, V any](seq Seq2[K, V]) (next func() (K, V, bool), stop func()) {
127147
var (
128-
k K
129-
v V
130-
ok bool
131-
done bool
132-
yieldNext bool
133-
racer int
148+
k K
149+
v V
150+
ok bool
151+
done bool
152+
yieldNext bool
153+
racer int
154+
panicValue any
134155
)
135156
c := newcoro(func(c *coro) {
136157
race.Acquire(unsafe.Pointer(&racer))
@@ -148,15 +169,23 @@ func Pull2[K, V any](seq Seq2[K, V]) (next func() (K, V, bool), stop func()) {
148169
race.Acquire(unsafe.Pointer(&racer))
149170
return !done
150171
}
172+
// Recover and propagate panics from seq.
173+
defer func() {
174+
if p := recover(); p != nil {
175+
done = true // Invalidate iterator.
176+
panicValue = p
177+
}
178+
race.Release(unsafe.Pointer(&racer))
179+
}()
151180
seq(yield)
152181
var k0 K
153182
var v0 V
154183
k, v, ok = k0, v0, false
155184
done = true
156-
race.Release(unsafe.Pointer(&racer))
157185
})
158186
next = func() (k1 K, v1 V, ok1 bool) {
159187
race.Write(unsafe.Pointer(&racer)) // detect races
188+
160189
if done {
161190
return
162191
}
@@ -167,15 +196,26 @@ func Pull2[K, V any](seq Seq2[K, V]) (next func() (K, V, bool), stop func()) {
167196
race.Release(unsafe.Pointer(&racer))
168197
coroswitch(c)
169198
race.Acquire(unsafe.Pointer(&racer))
199+
200+
// Propagate panics from seq.
201+
if panicValue != nil {
202+
panic(panicValue)
203+
}
170204
return k, v, ok
171205
}
172206
stop = func() {
173207
race.Write(unsafe.Pointer(&racer)) // detect races
208+
174209
if !done {
175210
done = true
176211
race.Release(unsafe.Pointer(&racer))
177212
coroswitch(c)
178213
race.Acquire(unsafe.Pointer(&racer))
214+
215+
// Propagate panics from seq.
216+
if panicValue != nil {
217+
panic(panicValue)
218+
}
179219
}
180220
}
181221
return next, stop

Diff for: src/iter/pull_test.go

+79
Original file line numberDiff line numberDiff line change
@@ -241,3 +241,82 @@ func storeYield2() Seq2[int, int] {
241241
}
242242

243243
var yieldSlot2 func(int, int) bool
244+
245+
func TestPullPanic(t *testing.T) {
246+
t.Run("next", func(t *testing.T) {
247+
next, stop := Pull(panicSeq())
248+
if !panicsWith("boom", func() { next() }) {
249+
t.Fatal("failed to propagate panic on first next")
250+
}
251+
// Make sure we don't panic again if we try to call next or stop.
252+
if _, ok := next(); ok {
253+
t.Fatal("next returned true after iterator panicked")
254+
}
255+
// Calling stop again should be a no-op.
256+
stop()
257+
})
258+
t.Run("stop", func(t *testing.T) {
259+
next, stop := Pull(panicSeq())
260+
if !panicsWith("boom", func() { stop() }) {
261+
t.Fatal("failed to propagate panic on first stop")
262+
}
263+
// Make sure we don't panic again if we try to call next or stop.
264+
if _, ok := next(); ok {
265+
t.Fatal("next returned true after iterator panicked")
266+
}
267+
// Calling stop again should be a no-op.
268+
stop()
269+
})
270+
}
271+
272+
func panicSeq() Seq[int] {
273+
return func(yield func(int) bool) {
274+
panic("boom")
275+
}
276+
}
277+
278+
func TestPull2Panic(t *testing.T) {
279+
t.Run("next", func(t *testing.T) {
280+
next, stop := Pull2(panicSeq2())
281+
if !panicsWith("boom", func() { next() }) {
282+
t.Fatal("failed to propagate panic on first next")
283+
}
284+
// Make sure we don't panic again if we try to call next or stop.
285+
if _, _, ok := next(); ok {
286+
t.Fatal("next returned true after iterator panicked")
287+
}
288+
// Calling stop again should be a no-op.
289+
stop()
290+
})
291+
t.Run("stop", func(t *testing.T) {
292+
next, stop := Pull2(panicSeq2())
293+
if !panicsWith("boom", func() { stop() }) {
294+
t.Fatal("failed to propagate panic on first stop")
295+
}
296+
// Make sure we don't panic again if we try to call next or stop.
297+
if _, _, ok := next(); ok {
298+
t.Fatal("next returned true after iterator panicked")
299+
}
300+
// Calling stop again should be a no-op.
301+
stop()
302+
})
303+
}
304+
305+
func panicSeq2() Seq2[int, int] {
306+
return func(yield func(int, int) bool) {
307+
panic("boom")
308+
}
309+
}
310+
311+
func panicsWith(v any, f func()) (panicked bool) {
312+
defer func() {
313+
if r := recover(); r != nil {
314+
if r != v {
315+
panic(r)
316+
}
317+
panicked = true
318+
}
319+
}()
320+
f()
321+
return
322+
}

0 commit comments

Comments
 (0)