From 498dd8098473c914c013c831759b549618312790 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Thu, 17 Sep 2026 22:30:32 -0400 Subject: [PATCH 1/2] pool: the errand tracker's critical sections unlock from a defer internal/guard's lock law failed on santos/dev since #1124: stopPoolErrands and poolErrandGo took poolErrandsMu without a deferred Unlock, so an absorbed panic inside either section would deadlock every later pool start-up. The two sections are their own functions now, each unlocking from a defer, and the wait and the goroutine start happen with the lock released as before. --- cmd/codeaf/poolindex.go | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/cmd/codeaf/poolindex.go b/cmd/codeaf/poolindex.go index 25a05f2def..a36d1e67d7 100644 --- a/cmd/codeaf/poolindex.go +++ b/cmd/codeaf/poolindex.go @@ -129,10 +129,7 @@ func poolErrandsStart(profileDir string) *poolErrands { // profile — is gone, which is a write into a directory nobody owns. It is // idempotent: a profile with no live errands is a no-op. func stopPoolErrands(profileDir string) { - poolErrandsMu.Lock() - held := poolErrandSet[profileDir] - delete(poolErrandSet, profileDir) - poolErrandsMu.Unlock() + held := takePoolErrands(profileDir) if held == nil { return } @@ -140,17 +137,23 @@ func stopPoolErrands(profileDir string) { held.wg.Wait() } +// takePoolErrands removes and returns the profile's tracker under the lock, so +// the wait that follows happens with the lock released: a Wait under the lock +// would hold every other profile's start-up behind one profile's shutdown. +func takePoolErrands(profileDir string) *poolErrands { + poolErrandsMu.Lock() + defer poolErrandsMu.Unlock() + held := poolErrandSet[profileDir] + delete(poolErrandSet, profileDir) + return held +} + // poolErrandGo starts one errand on the profile's tracker, so [stopPoolErrands] // waits for it, through the same [poolRefreshGo] seam every pool goroutine // starts through. A profile wired without a tracker — a bare // [startPoolIndexRefresh] in a test — is the plain seam with nothing to join. func poolErrandGo(profileDir, scope string, fn func()) { - poolErrandsMu.Lock() - held := poolErrandSet[profileDir] - if held != nil { - held.wg.Add(1) - } - poolErrandsMu.Unlock() + held := joinPoolErrands(profileDir) if held == nil { poolRefreshGo(scope, fn) return @@ -161,6 +164,18 @@ func poolErrandGo(profileDir, scope string, fn func()) { }) } +// joinPoolErrands counts one more errand on the profile's tracker under the +// lock and returns the tracker, or nil when the profile was wired without one. +func joinPoolErrands(profileDir string) *poolErrands { + poolErrandsMu.Lock() + defer poolErrandsMu.Unlock() + held := poolErrandSet[profileDir] + if held != nil { + held.wg.Add(1) + } + return held +} + // startPoolIndexRefresh is the loader's own tail: it starts the background // fetch that keeps the cache fresh, and starts NOTHING when the mode forbids // reading or the build carries no public key to check a fetched document under From 1104700108fc05bb006b2d19855a35ac211e5175 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Thu, 17 Sep 2026 22:30:35 -0400 Subject: [PATCH 2/2] changes: note #1134 --- .../1134-pool-errand-sections-unlock-from-a-defer.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 docs/changes/unreleased/1134-pool-errand-sections-unlock-from-a-defer.md diff --git a/docs/changes/unreleased/1134-pool-errand-sections-unlock-from-a-defer.md b/docs/changes/unreleased/1134-pool-errand-sections-unlock-from-a-defer.md new file mode 100644 index 0000000000..9d08b96119 --- /dev/null +++ b/docs/changes/unreleased/1134-pool-errand-sections-unlock-from-a-defer.md @@ -0,0 +1,8 @@ +--- +kind: internal +title: the pool errand tracker's critical sections unlock from a defer +pr: 1134 +surface: [engine] +invalidates: + - "`stopPoolErrands` and `poolErrandGo` in `cmd/codeaf/poolindex.go` took `poolErrandsMu` without a deferred unlock, which the guarded tree's lock law refuses because an absorbed panic inside the section would deadlock every later pool start-up. Each section is its own function now and unlocks from a defer; the wait and the goroutine start still happen with the lock released." +---