runtime: keep workers alive after main Goexit - #2259
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: keep workers alive after main Goexit
The reference-counting approach is sound in intent, and the NewProc error path and ordinary runtime-owned worker path are correctly balanced (exactly one retainG in initRuntimeContext, one releaseG in mexit). However, there is a deterministic double-free + gcount double-decrement on the exact scenario this PR targets — the main goroutine calling runtime.Goexit with a live peer — plus a cross-word race in the deadlock detection. See inline comments for the three concrete findings.
Why the double-free is deterministic (not a race): the main goroutine's g is created lazily via getg() → setAutoG, which sets currentGHasLifecycle = true (g_tls.go:91-98) and installs the pthread destructor. It is never re-installed via setg before Goexit. So when main reaches mexit, setg(nil) (proc.go:149) takes the currentGHasLifecycle branch and calls destroyG(old) on the same context — which frees the root and calls releaseG() a second time. The included test cl/_testgo/goexitmain/in.go only passes because the peer races to os.Exit(0) before the corruption surfaces.
Additional (non-inline) note — deadlock check duplicated across three sites: the same fatal("no goroutines (main called runtime.Goexit) - deadlock!"); c.Exit(2) is emitted from z_default.go:39-42, proc.go:135-139, and g_tls.go:105-109, using two different predicates (gp.isMain && liveGCount() == 1 vs releaseG() == 0 && hasMainExited()). Consider extracting a single helper so the invariants cannot drift, and note that if fatal(...) already terminates, the trailing c.Exit(2) is dead code.
Test suggestion: after fixing ownership, add a deterministic variant of goexitmain where the peer signals completion and lets the process exit normally (rather than calling os.Exit), so the double-release/double-free path is actually exercised instead of raced past.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Compared with |
651c6e7 to
0139545
Compare
2 participants
Fixes the scheduler-liveness root cause behind GOROOT
fixedbugs/issue5963.go.Summary
atomic.Subreturning the pre-subtraction valueruntime.Goexitto finish while another goroutine remains alive, while preserving the deadlock when main has no peer or the final worker returnsissue5963.goxfailsBare metal keeps its existing single-context behavior through documented no-op/sentinel helpers. This PR does not modify CI configuration or timeouts.
Tests
mexit, then lets the peer return normally; it verifies the worker marker precedes the last-goroutine deadlockgo test -vet=off ./test/go -run '^TestMainGoexitLifecycleReleasedOnce$' -count=1go run ./cmd/llgo test -run 'Goexit|GMP' ./test/std/runtime ./test/llgoextgo test ./ssa -run '^TestFromTestgo$' -count=1fixedbugs/issue5963.gopasses without xfail on Go 1.24.11, 1.25.0, and 1.26.5fixedbugs/issue8158.goandfixedbugs/issue11256.go