pool: the errand tracker's critical sections unlock from a defer

cmd/codeaf/poolindex.go

@@ -129,28 +129,31 @@ 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
 	}
 	held.cancel()
 	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

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