Fix activator double-unlock that crashed miren under activation contention

[phinze]

Garden hard-crashed with fatal error: sync: Unlock of unlocked RWMutex, which on a single-node miren means the whole process goes down: embedded etcd, every app, everything. A systemctl restart brought it back, but it's the kind of bug that bites again the moment traffic spikes.

The culprit lives in the activator's requestPoolCapacity. When no pool is cached yet, it runs a small retry loop that polls the entity store for a pool the DeploymentLauncher might be creating concurrently. That inner loop has an invariant: a.mu is held at the top of every iteration, because the backoff between attempts unlocks the mutex during the sleep and re-locks before the next store query. One branch broke it. When a goroutine noticed another goroutine had already populated the cache, it unlocked and did a bare continue, meaning to loop back to the outer wait/increment logic. But continue only targets the nearest enclosing loop, so it re-entered the inner loop with the lock released. The next iteration's backoff-unlock then fired on an already-unlocked mutex, and Go turns that into a fatal throw you can't recover from.

It's a narrow race: it only triggers when a concurrent goroutine wins the pool-creation race specifically on a retry attempt, so you need real activation contention (lots of apps waking at once) to hit it. That's why it stayed hidden since October and only showed up now. #883's pool-eviction work increased pool churn and pushed enough traffic through this path to finally surface it.

The fix labels the outer loop and makes that branch continue poolLoop, so it breaks all the way out and picks the pool up through the normal cached path with the lock in the state the outer loop expects (released). While in here I audited the rest of the retry loop's lock discipline; this was the only actual defect, but I gave the sibling branch just above it the same explicit label (it was already correct by accident of nesting) and wrote the invariant down in a comment so the next person doesn't have to re-derive it. There's a new stress test that reconstructs the race by making a pool appear in the store while a crowd of callers is mid-backoff.

One thing worth flagging separately: running that stress test under -race also turns up a pre-existing data race on the increment path (concurrent access to the shared cached pool state), unrelated to this crash and untouched by this change. The normal suite doesn't run -race, so it's not blocking, but it's real and I'll file a follow-up.

Closes MIR-1306

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6fbbc2dd-16e7-42ce-903f-8f47064d4a0f

📥 Commits

Reviewing files that changed from the base of the PR and between e2cfcbf and 6c2972a.

📒 Files selected for processing (2)

• components/activator/activator.go
• components/activator/activator_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• components/activator/activator_test.go
• components/activator/activator.go

---

📝 Walkthrough

Walkthrough

This change updates localActivator.requestPoolCapacity to use a labeled outer loop and explicit lock-invariant comments in its retry path. When another goroutine has already created or found the pool, the code now unlocks and continues via poolLoop instead of an unlabeled continue. A new regression test runs repeated high-concurrency calls against an etcd-backed store to exercise the race and confirm the process does not crash.

Changes

Cohort / File(s)	Summary
Double-unlock race fix components/activator/activator.go	Adds labeled poolLoop, lock-invariant comments, and replaces unlabeled continue with continue poolLoop in the retry path
components/activator/activator_test.go	Adds fmt import and TestRequestPoolCapacityRetryRaceNoDoubleUnlock

Sequence Diagram(s)

sequenceDiagram
  participant Goroutine
  participant requestPoolCapacity
  participant Mutex
  participant EntityStore

  Goroutine->>requestPoolCapacity: call requestPoolCapacity
  requestPoolCapacity->>Mutex: lock
  requestPoolCapacity->>EntityStore: lookup/create pool
  alt pool already exists
    requestPoolCapacity->>Mutex: unlock
    requestPoolCapacity->>requestPoolCapacity: continue poolLoop
  else pool missing
    requestPoolCapacity->>Mutex: unlock
    requestPoolCapacity->>requestPoolCapacity: retry after backoff
  end
  requestPoolCapacity-->>Goroutine: return

Loading

Related Issues: MIR-1306

Related PRs: None mentioned

Suggested labels: bug, concurrency, regression-test

Suggested reviewers: None specified

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@components/activator/activator_test.go`:
- Around line 3691-3697: The example command in
TestRequestPoolCapacityRetryRaceNoDoubleUnlock is misleading because it
recommends running with -race even though the known increment-path race will
trip the detector and fail the test. Update the test comment near the retry-race
scenario to either remove -race from the documented ./hack/dev-exec go test
invocation or explicitly note that the race check is intentionally deferred, and
if needed add a guard in the test itself to skip or avoid the race-detector path
when running under -race.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a19ff5e2-c30b-41f7-addc-f72b72d9b0c6

📥 Commits

Reviewing files that changed from the base of the PR and between 9829b72 and e2cfcbf.

📒 Files selected for processing (2)

• components/activator/activator.go
• components/activator/activator_test.go