]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: avoid deadlock in synctest changegstatus when copying stacks
authorDamien Neil <dneil@google.com>
Tue, 19 Nov 2024 22:41:37 +0000 (14:41 -0800)
committerGopher Robot <gobot@golang.org>
Wed, 20 Nov 2024 16:45:48 +0000 (16:45 +0000)
For #67434
Fixes #70452

Change-Id: Ie655a9e55837aa68b6bfb0bb69b6c8caaf3bbea5
Reviewed-on: https://go-review.googlesource.com/c/go/+/629856
Reviewed-by: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
src/runtime/synctest.go

index b4934de8530d140e9d23cd52049d0c6de991d24c..0fd5e7873e4f5df12a98956568383613a51f8574 100644 (file)
@@ -36,12 +36,20 @@ type synctestGroup struct {
 // changegstatus is called when the non-lock status of a g changes.
 // It is never called with a Gscanstatus.
 func (sg *synctestGroup) changegstatus(gp *g, oldval, newval uint32) {
-       lock(&sg.mu)
+       // Determine whether this change in status affects the idleness of the group.
+       // If this isn't a goroutine starting, stopping, durably blocking,
+       // or waking up after durably blocking, then return immediately without
+       // locking sg.mu.
+       //
+       // For example, stack growth (newstack) will changegstatus
+       // from _Grunning to _Gcopystack. This is uninteresting to synctest,
+       // but if stack growth occurs while sg.mu is held, we must not recursively lock.
+       totalDelta := 0
        wasRunning := true
        switch oldval {
        case _Gdead:
                wasRunning = false
-               sg.total++
+               totalDelta++
        case _Gwaiting:
                if gp.waitreason.isIdleInSynctest() {
                        wasRunning = false
@@ -51,12 +59,20 @@ func (sg *synctestGroup) changegstatus(gp *g, oldval, newval uint32) {
        switch newval {
        case _Gdead:
                isRunning = false
-               sg.total--
+               totalDelta--
        case _Gwaiting:
                if gp.waitreason.isIdleInSynctest() {
                        isRunning = false
                }
        }
+       // It's possible for wasRunning == isRunning while totalDelta != 0;
+       // for example, if a new goroutine is created in a non-running state.
+       if wasRunning == isRunning && totalDelta == 0 {
+               return
+       }
+
+       lock(&sg.mu)
+       sg.total += totalDelta
        if wasRunning != isRunning {
                if isRunning {
                        sg.running++