Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit a8564bd

Browse files
neildgopherbot
authored andcommitted
runtime: make all synctest bubble violations fatal panics
Unblocking a bubbled goroutine from outside the bubble is an error and panics. Currently, some of those panics are regular panics and some are fatal. We use fatal panics in cases where its difficult to panic without leaving something in an inconsistent state. Change the regular panics (channel and timer operations) to be fatal. This makes our behavior more consistent: All bubble violations are always fatal. More importantly, it avoids introducing new, recoverable panics. A motivating example for this change is the context package, which performs channel operations with a mutex held in the expectation that those operations can never panic. These operations can now panic as a result of a bubble violation, potentially leaving a context.Context in an inconsistent state. Fixes #74837 Change-Id: Ie6efd916b7f505c0f13dde42de1572992401f15c Reviewed-on: https://go-review.googlesource.com/c/go/+/696195 Auto-Submit: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
1 parent 924fe98 commit a8564bd

File tree

4 files changed

+51
-48
lines changed

4 files changed

+51
-48
lines changed

‎src/internal/synctest/synctest_test.go

Lines changed: 43 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -383,57 +383,59 @@ func TestChannelMovedOutOfBubble(t *testing.T) {
383383
for _, test := range []struct {
384384
desc string
385385
f func(chan struct{})
386-
wantPanic string
386+
wantFatal string
387387
}{{
388388
desc: "receive",
389389
f: func(ch chan struct{}) {
390390
<-ch
391391
},
392-
wantPanic: "receive on synctest channel from outside bubble",
392+
wantFatal: "receive on synctest channel from outside bubble",
393393
}, {
394394
desc: "send",
395395
f: func(ch chan struct{}) {
396396
ch <- struct{}{}
397397
},
398-
wantPanic: "send on synctest channel from outside bubble",
398+
wantFatal: "send on synctest channel from outside bubble",
399399
}, {
400400
desc: "close",
401401
f: func(ch chan struct{}) {
402402
close(ch)
403403
},
404-
wantPanic: "close of synctest channel from outside bubble",
404+
wantFatal: "close of synctest channel from outside bubble",
405405
}} {
406406
t.Run(test.desc, func(t *testing.T) {
407407
// Bubbled channel accessed from outside any bubble.
408408
t.Run("outside_bubble", func(t *testing.T) {
409-
donec := make(chan struct{})
410-
ch := make(chan chan struct{})
411-
go func() {
412-
defer close(donec)
413-
defer wantPanic(t, test.wantPanic)
414-
test.f(<-ch)
415-
}()
416-
synctest.Run(func() {
417-
ch <- make(chan struct{})
409+
wantFatal(t, test.wantFatal, func() {
410+
donec := make(chan struct{})
411+
ch := make(chan chan struct{})
412+
go func() {
413+
defer close(donec)
414+
test.f(<-ch)
415+
}()
416+
synctest.Run(func() {
417+
ch <- make(chan struct{})
418+
})
419+
<-donec
418420
})
419-
<-donec
420421
})
421422
// Bubbled channel accessed from a different bubble.
422423
t.Run("different_bubble", func(t *testing.T) {
423-
donec := make(chan struct{})
424-
ch := make(chan chan struct{})
425-
go func() {
426-
defer close(donec)
427-
c := <-ch
424+
wantFatal(t, test.wantFatal, func() {
425+
donec := make(chan struct{})
426+
ch := make(chan chan struct{})
427+
go func() {
428+
defer close(donec)
429+
c := <-ch
430+
synctest.Run(func() {
431+
test.f(c)
432+
})
433+
}()
428434
synctest.Run(func() {
429-
defer wantPanic(t, test.wantPanic)
430-
test.f(c)
435+
ch <- make(chan struct{})
431436
})
432-
}()
433-
synctest.Run(func() {
434-
ch <- make(chan struct{})
437+
<-donec
435438
})
436-
<-donec
437439
})
438440
})
439441
}
@@ -443,39 +445,40 @@ func TestTimerFromInsideBubble(t *testing.T) {
443445
for _, test := range []struct {
444446
desc string
445447
f func(tm *time.Timer)
446-
wantPanic string
448+
wantFatal string
447449
}{{
448450
desc: "read channel",
449451
f: func(tm *time.Timer) {
450452
<-tm.C
451453
},
452-
wantPanic: "receive on synctest channel from outside bubble",
454+
wantFatal: "receive on synctest channel from outside bubble",
453455
}, {
454456
desc: "Reset",
455457
f: func(tm *time.Timer) {
456458
tm.Reset(1 * time.Second)
457459
},
458-
wantPanic: "reset of synctest timer from outside bubble",
460+
wantFatal: "reset of synctest timer from outside bubble",
459461
}, {
460462
desc: "Stop",
461463
f: func(tm *time.Timer) {
462464
tm.Stop()
463465
},
464-
wantPanic: "stop of synctest timer from outside bubble",
466+
wantFatal: "stop of synctest timer from outside bubble",
465467
}} {
466468
t.Run(test.desc, func(t *testing.T) {
467-
donec := make(chan struct{})
468-
ch := make(chan *time.Timer)
469-
go func() {
470-
defer close(donec)
471-
defer wantPanic(t, test.wantPanic)
472-
test.f(<-ch)
473-
}()
474-
synctest.Run(func() {
475-
tm := time.NewTimer(1 * time.Second)
476-
ch <- tm
469+
wantFatal(t, test.wantFatal, func() {
470+
donec := make(chan struct{})
471+
ch := make(chan *time.Timer)
472+
go func() {
473+
defer close(donec)
474+
test.f(<-ch)
475+
}()
476+
synctest.Run(func() {
477+
tm := time.NewTimer(1 * time.Second)
478+
ch <- tm
479+
})
480+
<-donec
477481
})
478-
<-donec
479482
})
480483
}
481484
}

‎src/runtime/chan.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ func chansend(c *hchan, ep unsafe.Pointer, block bool, callerpc uintptr) bool {
191191
}
192192

193193
if c.bubble != nil && getg().bubble != c.bubble {
194-
panic(plainError("send on synctest channel from outside bubble"))
194+
fatal("send on synctest channel from outside bubble")
195195
}
196196

197197
// Fast path: check for failed non-blocking operation without acquiring the lock.
@@ -318,7 +318,7 @@ func chansend(c *hchan, ep unsafe.Pointer, block bool, callerpc uintptr) bool {
318318
func send(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) {
319319
if c.bubble != nil && getg().bubble != c.bubble {
320320
unlockf()
321-
panic(plainError("send on synctest channel from outside bubble"))
321+
fatal("send on synctest channel from outside bubble")
322322
}
323323
if raceenabled {
324324
if c.dataqsiz == 0 {
@@ -416,7 +416,7 @@ func closechan(c *hchan) {
416416
panic(plainError("close of nil channel"))
417417
}
418418
if c.bubble != nil && getg().bubble != c.bubble {
419-
panic(plainError("close of synctest channel from outside bubble"))
419+
fatal("close of synctest channel from outside bubble")
420420
}
421421

422422
lock(&c.lock)
@@ -538,7 +538,7 @@ func chanrecv(c *hchan, ep unsafe.Pointer, block bool) (selected, received bool)
538538
}
539539

540540
if c.bubble != nil && getg().bubble != c.bubble {
541-
panic(plainError("receive on synctest channel from outside bubble"))
541+
fatal("receive on synctest channel from outside bubble")
542542
}
543543

544544
if c.timer != nil {
@@ -702,7 +702,7 @@ func chanrecv(c *hchan, ep unsafe.Pointer, block bool) (selected, received bool)
702702
func recv(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) {
703703
if c.bubble != nil && getg().bubble != c.bubble {
704704
unlockf()
705-
panic(plainError("receive on synctest channel from outside bubble"))
705+
fatal("receive on synctest channel from outside bubble")
706706
}
707707
if c.dataqsiz == 0 {
708708
if raceenabled {

‎src/runtime/select.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func selectgo(cas0 *scase, order0 *uint16, pc0 *uintptr, nsends, nrecvs int, blo
178178

179179
if cas.c.bubble != nil {
180180
if getg().bubble != cas.c.bubble {
181-
panic(plainError("select on synctest channel from outside bubble"))
181+
fatal("select on synctest channel from outside bubble")
182182
}
183183
} else {
184184
allSynctest = false

‎src/runtime/time.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ func newTimer(when, period int64, f func(arg any, seq uintptr, delay int64), arg
415415
//go:linkname stopTimer time.stopTimer
416416
func stopTimer(t *timeTimer) bool {
417417
if t.isFake && getg().bubble == nil {
418-
panic("stop of synctest timer from outside bubble")
418+
fatal("stop of synctest timer from outside bubble")
419419
}
420420
return t.stop()
421421
}
@@ -430,7 +430,7 @@ func resetTimer(t *timeTimer, when, period int64) bool {
430430
racerelease(unsafe.Pointer(&t.timer))
431431
}
432432
if t.isFake && getg().bubble == nil {
433-
panic("reset of synctest timer from outside bubble")
433+
fatal("reset of synctest timer from outside bubble")
434434
}
435435
return t.reset(when, period)
436436
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /