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 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." +---