You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Cold resume can overcommit a runner's capacity when two resumes race #30
Cold resume can overcommit a runner's capacity when two resumes race
SessionService.ResumeSession's cold-resume branch checks coldResumeFree (capacity total minus used minus anything still creating on the runner) and, if it sees a free slot, dispatches the resume and transitions the session straight from suspended_cold to running. It never passes through creating, so a concurrent resume reading the same runner in that window sees the same free slot and does the same thing.
I hit this with two sessions pinned to one runner that has exactly one free slot: both ResumeSession calls read coldResumeFree() == 1, both dispatch, both land in running. The runner ends up hosting one more session than its CapacityTotal allows.
The normal placement path doesn't have this gap: FleetService's scheduler transitions a session to creating (an atomic CAS via Transition, guarded by the session's current state) before it dispatches, so freeCapacity's len(creating) count reflects the claim for anyone reading after it. Cold resume skips that intermediate state entirely, so there's nothing for a second reader to see.
I reproduced it with a unit test driving the real SessionService against fakes for the two I/O ports it touches (FleetRepository, SessionRepository), with a barrier forcing both goroutines through the capacity read before either commits:
resumes that succeeded: 2 (runner had 1 free slot)
resume commands actually dispatched to the runner: 2
sess_a final state: running
sess_b final state: running
Race-detector clean across 5 runs; it's a logical race in the check-then-act sequence, not a Go memory race.
Two ways I can see to close it, and I don't want to guess which fits the rest of the fleet code better:
run the cold-resume dispatch through the same creating reservation the placement path already uses
a per-runner lock around the check-dispatch-transition sequence, similar to what nameLock does in internal/controld/runners.go for the connection-lifecycle writes, scoped to controlapp instead
Happy to send a PR either way once you've got a preference, or if there's a reservation mechanism already planned for controlapp that I'm not seeing.
Activity