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>

diff --git a/src/internal/synctest/synctest_test.go b/src/internal/synctest/synctest_test.go
index 6cebf86c31f4168fc9a053ed97a254fe2f8b26eb..307eee62e2b9aa0096ce9b98fd5bb2bae2556b6d 100644 (file)
--- a/src/internal/synctest/synctest_test.go
+++ b/src/internal/synctest/synctest_test.go
@@ -383,57 +383,59 @@ func TestChannelMovedOutOfBubble(t *testing.T) {
        for _, test := range []struct {
                desc      string
                f         func(chan struct{})
-               wantPanic string
+               wantFatal string
        }{{
                desc: "receive",
                f: func(ch chan struct{}) {
                        <-ch
                },
-               wantPanic: "receive on synctest channel from outside bubble",
+               wantFatal: "receive on synctest channel from outside bubble",
        }, {
                desc: "send",
                f: func(ch chan struct{}) {
                        ch <- struct{}{}
                },
-               wantPanic: "send on synctest channel from outside bubble",
+               wantFatal: "send on synctest channel from outside bubble",
        }, {
                desc: "close",
                f: func(ch chan struct{}) {
                        close(ch)
                },
-               wantPanic: "close of synctest channel from outside bubble",
+               wantFatal: "close of synctest channel from outside bubble",
        }} {
                t.Run(test.desc, func(t *testing.T) {
                        // Bubbled channel accessed from outside any bubble.
                        t.Run("outside_bubble", func(t *testing.T) {
-                               donec := make(chan struct{})
-                               ch := make(chan chan struct{})
-                               go func() {
-                                       defer close(donec)
-                                       defer wantPanic(t, test.wantPanic)
-                                       test.f(<-ch)
-                               }()
-                               synctest.Run(func() {
-                                       ch <- make(chan struct{})
+                               wantFatal(t, test.wantFatal, func() {
+                                       donec := make(chan struct{})
+                                       ch := make(chan chan struct{})
+                                       go func() {
+                                               defer close(donec)
+                                               test.f(<-ch)
+                                       }()
+                                       synctest.Run(func() {
+                                               ch <- make(chan struct{})
+                                       })
+                                       <-donec
                                })
-                               <-donec
                        })
                        // Bubbled channel accessed from a different bubble.
                        t.Run("different_bubble", func(t *testing.T) {
-                               donec := make(chan struct{})
-                               ch := make(chan chan struct{})
-                               go func() {
-                                       defer close(donec)
-                                       c := <-ch
+                               wantFatal(t, test.wantFatal, func() {
+                                       donec := make(chan struct{})
+                                       ch := make(chan chan struct{})
+                                       go func() {
+                                               defer close(donec)
+                                               c := <-ch
+                                               synctest.Run(func() {
+                                                       test.f(c)
+                                               })
+                                       }()
                                        synctest.Run(func() {
-                                               defer wantPanic(t, test.wantPanic)
-                                               test.f(c)
+                                               ch <- make(chan struct{})
                                        })
-                               }()
-                               synctest.Run(func() {
-                                       ch <- make(chan struct{})
+                                       <-donec
                                })
-                               <-donec
                        })
                })
        }
@@ -443,39 +445,40 @@ func TestTimerFromInsideBubble(t *testing.T) {
        for _, test := range []struct {
                desc      string
                f         func(tm *time.Timer)
-               wantPanic string
+               wantFatal string
        }{{
                desc: "read channel",
                f: func(tm *time.Timer) {
                        <-tm.C
                },
-               wantPanic: "receive on synctest channel from outside bubble",
+               wantFatal: "receive on synctest channel from outside bubble",
        }, {
                desc: "Reset",
                f: func(tm *time.Timer) {
                        tm.Reset(1 * time.Second)
                },
-               wantPanic: "reset of synctest timer from outside bubble",
+               wantFatal: "reset of synctest timer from outside bubble",
        }, {
                desc: "Stop",
                f: func(tm *time.Timer) {
                        tm.Stop()
                },
-               wantPanic: "stop of synctest timer from outside bubble",
+               wantFatal: "stop of synctest timer from outside bubble",
        }} {
                t.Run(test.desc, func(t *testing.T) {
-                       donec := make(chan struct{})
-                       ch := make(chan *time.Timer)
-                       go func() {
-                               defer close(donec)
-                               defer wantPanic(t, test.wantPanic)
-                               test.f(<-ch)
-                       }()
-                       synctest.Run(func() {
-                               tm := time.NewTimer(1 * time.Second)
-                               ch <- tm
+                       wantFatal(t, test.wantFatal, func() {
+                               donec := make(chan struct{})
+                               ch := make(chan *time.Timer)
+                               go func() {
+                                       defer close(donec)
+                                       test.f(<-ch)
+                               }()
+                               synctest.Run(func() {
+                                       tm := time.NewTimer(1 * time.Second)
+                                       ch <- tm
+                               })
+                               <-donec
                        })
-                       <-donec
                })
        }
 }

diff --git a/src/runtime/chan.go b/src/runtime/chan.go
index bb554ebfdb1f3a0f9d96df19adce1248f23c9d70..639d29dc8337f00d2655c76b8910565be6f383d5 100644 (file)
--- a/src/runtime/chan.go
+++ b/src/runtime/chan.go
@@ -191,7 +191,7 @@ func chansend(c *hchan, ep unsafe.Pointer, block bool, callerpc uintptr) bool {
        }
 
        if c.bubble != nil && getg().bubble != c.bubble {
-               panic(plainError("send on synctest channel from outside bubble"))
+               fatal("send on synctest channel from outside bubble")
        }
 
        // 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 {
 func send(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) {
        if c.bubble != nil && getg().bubble != c.bubble {
                unlockf()
-               panic(plainError("send on synctest channel from outside bubble"))
+               fatal("send on synctest channel from outside bubble")
        }
        if raceenabled {
                if c.dataqsiz == 0 {
@@ -416,7 +416,7 @@ func closechan(c *hchan) {
                panic(plainError("close of nil channel"))
        }
        if c.bubble != nil && getg().bubble != c.bubble {
-               panic(plainError("close of synctest channel from outside bubble"))
+               fatal("close of synctest channel from outside bubble")
        }
 
        lock(&c.lock)
@@ -538,7 +538,7 @@ func chanrecv(c *hchan, ep unsafe.Pointer, block bool) (selected, received bool)
        }
 
        if c.bubble != nil && getg().bubble != c.bubble {
-               panic(plainError("receive on synctest channel from outside bubble"))
+               fatal("receive on synctest channel from outside bubble")
        }
 
        if c.timer != nil {
@@ -702,7 +702,7 @@ func chanrecv(c *hchan, ep unsafe.Pointer, block bool) (selected, received bool)
 func recv(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) {
        if c.bubble != nil && getg().bubble != c.bubble {
                unlockf()
-               panic(plainError("receive on synctest channel from outside bubble"))
+               fatal("receive on synctest channel from outside bubble")
        }
        if c.dataqsiz == 0 {
                if raceenabled {

diff --git a/src/runtime/select.go b/src/runtime/select.go
index ae7754b17377ddc254ce55b5779ea0a62c46fe3b..113dc8ad19e984c371f3a6f437ba2ce14b97bb7d 100644 (file)
--- a/src/runtime/select.go
+++ b/src/runtime/select.go
@@ -178,7 +178,7 @@ func selectgo(cas0 *scase, order0 *uint16, pc0 *uintptr, nsends, nrecvs int, blo
 
                if cas.c.bubble != nil {
                        if getg().bubble != cas.c.bubble {
-                               panic(plainError("select on synctest channel from outside bubble"))
+                               fatal("select on synctest channel from outside bubble")
                        }
                } else {
                        allSynctest = false

diff --git a/src/runtime/time.go b/src/runtime/time.go
index 4880dce8cddc799f91c1d7b0b88f624aa28c0eb1..e9d1f0b6c9a10f90bccb6bf8575239255f82a7be 100644 (file)
--- a/src/runtime/time.go
+++ b/src/runtime/time.go
@@ -415,7 +415,7 @@ func newTimer(when, period int64, f func(arg any, seq uintptr, delay int64), arg
 //go:linkname stopTimer time.stopTimer
 func stopTimer(t *timeTimer) bool {
        if t.isFake && getg().bubble == nil {
-               panic("stop of synctest timer from outside bubble")
+               fatal("stop of synctest timer from outside bubble")
        }
        return t.stop()
 }
@@ -430,7 +430,7 @@ func resetTimer(t *timeTimer, when, period int64) bool {
                racerelease(unsafe.Pointer(&t.timer))
        }
        if t.isFake && getg().bubble == nil {
-               panic("reset of synctest timer from outside bubble")
+               fatal("reset of synctest timer from outside bubble")
        }
        return t.reset(when, period)
 }