Submit an issue View all issues Source
MIR-1306

Activator double-unlock (RWMutex.Unlock of unlocked) crashes miren on pool-lookup retry race

Done public
phinze phinze Opened Jul 6, 2026 Updated Jul 6, 2026

Garden (miren-development) hard-crashed at 2026-07-06 18:16:00 UTC with:

fatal error: sync: Unlock of unlocked RWMutex
goroutine 51225 [running]:
  sync/rwmutex.go:212
  components/activator/activator.go:853
  components/activator/activator.go:358
  components/activator/activator.go:317
  servers/httpingress/httpingress.go:769 (:572 :579 :469 :410)
  net/http … (incoming ingress request)

The crash is in the pool-lookup retry loop in getOrCreatePool (around activator.go:835-895). The loop acquires a.mu before it starts and relies on the invariant that the lock is held at the top of every iteration (line 853 does a.mu.Unlock() during the backoff sleep). But the "another goroutine already created the pool" branch inside the if attempt > 0 block does:

a.mu.Lock()
_, exists = a.pools[key]
if exists {
    a.mu.Unlock()
    continue // "Loop back to main logic"
}

That bare continue re-enters the attempt loop, not the outer loop the comment intends. On the next iteration attempt > 0 is still true, so it hits a.mu.Unlock() at line 853 on an already-unlocked mutex → fatal. It only fires when a concurrent goroutine wins the pool-creation race on a retry attempt, i.e. under activation contention (many apps waking at once).

Trigger: Surfaced right after #883 ("stop the activator stranding ingress...") deployed to garden at 17:12 UTC (garden version main:5b9b37b, built 17:12:29). #883's pool-eviction changes ("Evict stale version→pool bindings when a pool is dereferenced") increase pool churn and drive more traffic through this racy path. The double-unlock structure itself is older (blame dates it to late October), so #883 is the trigger rather than the author of the bug.

Fix direction: the if exists branch needs to either re-Lock() before continue, or break out to the outer loop (labeled continue), so the loop invariant holds. Worth an audit of the whole retry loop's lock discipline while we're in there.

Impact: whole-process fatal (takes down embedded etcd + all apps on the node). Recovered by systemctl restart miren. On garden this happened alongside a second concurrency crash the same day (index-watch send on closed channel, filed separately) and a follow-on context canceled storm against etcd that the restart cleared.