]> Cypherpunks repositories - gostls13.git/commitdiff
runtime, testing/synctest: breaking bubble isolation with Cond is fatal
authorDamien Neil <dneil@google.com>
Thu, 22 May 2025 18:14:53 +0000 (11:14 -0700)
committerGopher Robot <gobot@golang.org>
Thu, 29 May 2025 19:49:36 +0000 (12:49 -0700)
sync.Cond.Wait is durably blocking. Waking a goroutine out of Cond.Wait
from outside its bubble panics.

Make this panic a fatal panic, since it leaves the notifyList in an
inconsistent state. We could do some work to make this a recoverable
panic, but the complexity doesn't seem worth the outcome.

For #67434

Change-Id: I88874c1519c2e5c0063175297a9b120cedabcd07
Reviewed-on: https://go-review.googlesource.com/c/go/+/675617
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Damien Neil <dneil@google.com>

src/runtime/crash_test.go
src/runtime/sema.go
src/runtime/testdata/testprog/synctest.go [new file with mode: 0644]
src/testing/synctest/synctest.go

index 221a9a95cc21f8fb1f8321d04f8197e147bc9883..8696672065c26a999ef338e16f9bf34b2ba5b19d 100644 (file)
@@ -1228,3 +1228,20 @@ func TestFinalizerOrCleanupDeadlock(t *testing.T) {
                })
        }
 }
+
+func TestSynctestCondSignalFromNoBubble(t *testing.T) {
+       for _, test := range []string{
+               "SynctestCond/signal/no_bubble",
+               "SynctestCond/broadcast/no_bubble",
+               "SynctestCond/signal/other_bubble",
+               "SynctestCond/broadcast/other_bubble",
+       } {
+               t.Run(test, func(t *testing.T) {
+                       output := runTestProg(t, "testprog", test)
+                       want := "fatal error: semaphore wake of synctest goroutine from outside bubble"
+                       if !strings.Contains(output, want) {
+                               t.Fatalf("output:\n%s\n\nwant output containing: %s", output, want)
+                       }
+               })
+       }
+}
index 7d6fc6d57d9309d52c6f0d64c26cad789f6038f0..0f029f604fd537a15146eb71fdad412e11baf7d6 100644 (file)
@@ -635,7 +635,7 @@ func notifyListNotifyAll(l *notifyList) {
                s.next = nil
                if s.g.bubble != nil && getg().bubble != s.g.bubble {
                        println("semaphore wake of synctest goroutine", s.g.goid, "from outside bubble")
-                       panic("semaphore wake of synctest goroutine from outside bubble")
+                       fatal("semaphore wake of synctest goroutine from outside bubble")
                }
                readyWithTime(s, 4)
                s = next
@@ -692,7 +692,7 @@ func notifyListNotifyOne(l *notifyList) {
                        s.next = nil
                        if s.g.bubble != nil && getg().bubble != s.g.bubble {
                                println("semaphore wake of synctest goroutine", s.g.goid, "from outside bubble")
-                               panic("semaphore wake of synctest goroutine from outside bubble")
+                               fatal("semaphore wake of synctest goroutine from outside bubble")
                        }
                        readyWithTime(s, 4)
                        return
diff --git a/src/runtime/testdata/testprog/synctest.go b/src/runtime/testdata/testprog/synctest.go
new file mode 100644 (file)
index 0000000..dd3a6df
--- /dev/null
@@ -0,0 +1,58 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+       "internal/synctest"
+       "sync"
+)
+
+func init() {
+       register("SynctestCond/signal/no_bubble", func() {
+               synctestCond(func(cond *sync.Cond) {
+                       cond.Signal()
+               })
+       })
+       register("SynctestCond/broadcast/no_bubble", func() {
+               synctestCond(func(cond *sync.Cond) {
+                       cond.Broadcast()
+               })
+       })
+       register("SynctestCond/signal/other_bubble", func() {
+               synctestCond(func(cond *sync.Cond) {
+                       synctest.Run(cond.Signal)
+               })
+       })
+       register("SynctestCond/broadcast/other_bubble", func() {
+               synctestCond(func(cond *sync.Cond) {
+                       synctest.Run(cond.Broadcast)
+               })
+       })
+}
+
+func synctestCond(f func(*sync.Cond)) {
+       var (
+               mu     sync.Mutex
+               cond   = sync.NewCond(&mu)
+               readyc = make(chan struct{})
+               wg     sync.WaitGroup
+       )
+       defer wg.Wait()
+       wg.Go(func() {
+               synctest.Run(func() {
+                       go func() {
+                               mu.Lock()
+                               defer mu.Unlock()
+                               cond.Wait()
+                       }()
+                       synctest.Wait()
+                       <-readyc // #1: signal that cond.Wait is waiting
+                       <-readyc // #2: wait to continue
+                       cond.Signal()
+               })
+       })
+       readyc <- struct{}{}
+       f(cond)
+}
index 1664cb8484a5c19c5b3e141114d440dc04d73c13..a44047a9717550475f39b63d1b80de20bd56a657 100644 (file)
 //
 // A [sync.WaitGroup] becomes associated with a bubble on the first
 // call to Add or Go. Once a WaitGroup is associated with a bubble,
-// calling Add or Go from outside that bubble panics.
+// calling Add or Go from outside that bubble is a fatal error.
+//
+// [sync.Cond.Wait] is durably blocking. Waking a goroutine in a bubble
+// blocked on Cond.Wait from outside the bubble is a fatal error.
 //
 // Cleanup functions and finalizers registered with
 // [runtime.AddCleanup] and [runtime.SetFinalizer]