naps62/maestro
Archived
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

Guard the resume path against a concurrent kill and a concurrent resume #175

New Issue
Closed
opened 2026-09-04 09:06:49 +01:00 by naps62-yolo · 0 comments
naps62-yolo commented 2026-09-04 09:06:49 +01:00
Collaborator
Copy Link
Copy Source

Two races on the resume path, both from taking and releasing the session-map lock more than once.

A kill landing mid-resume leaves the sidecar behind. resume writes the sidecar before re-taking the map lock. If kill runs in that window, cleanup_record deletes the sidecar first, then resume's write puts it back. Resume itself then finds no record, kills the pane it just made and returns NotFound — but the sidecar it resurrected stays on disk, so the next recover registers a ghost Dormant session for a session that was killed and whose worktree is already gone. Fix: delete the sidecar in the branch that handles the record having disappeared.

Two concurrent resumes both proceed. The Running check releases the lock before the spawn, so two requests both pass it. Both open a PtyBridge on the same FIFO and compete for the pipe-pane bytes; the loser reports 502 tmux instead of the 409 already_running it should get. Fix: mark the record in-flight under the first lock so the second request is refused on the state, not on the spawn failing.

Cover both with tests. Found by the wave-2 integration review of the blitz/reboot-resilience branch.

Two races on the resume path, both from taking and releasing the session-map lock more than once. **A kill landing mid-resume leaves the sidecar behind.** `resume` writes the sidecar before re-taking the map lock. If `kill` runs in that window, `cleanup_record` deletes the sidecar first, then resume's write puts it back. Resume itself then finds no record, kills the pane it just made and returns `NotFound` — but the sidecar it resurrected stays on disk, so the next `recover` registers a ghost `Dormant` session for a session that was killed and whose worktree is already gone. Fix: delete the sidecar in the branch that handles the record having disappeared. **Two concurrent resumes both proceed.** The `Running` check releases the lock before the spawn, so two requests both pass it. Both open a `PtyBridge` on the same FIFO and compete for the `pipe-pane` bytes; the loser reports `502 tmux` instead of the `409 already_running` it should get. Fix: mark the record in-flight under the first lock so the second request is refused on the state, not on the spawn failing. Cover both with tests. Found by the wave-2 integration review of the `blitz/reboot-resilience` branch.
naps62-yolo added this to the Reboot resilience milestone 2026-09-04 09:06:49 +01:00
naps62-yolo added the difficulty/mediumtype/fixarea/daemon labels 2026-09-04 09:06:49 +01:00
naps62-yolo added a new dependency 2026-09-04 09:06:49 +01:00
#167 Add a resume_on_boot policy for recovered sessions
This repo is archived. You cannot comment on issues.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area/android
area/cli
area/config
area/daemon
area/protocol
area/tui
area/web
code-review
difficulty/easy
difficulty/hard
difficulty/medium
epic
Tracks a multi-issue epic / parent work item
 tui
type/docs
type/feat
type/fix
type/refactor
type/test
No labels area/daemon difficulty/medium type/fix
Milestone
No items
No Milestone Reboot resilience
Projects
Clear projects
No projects
Assignees
Clear assignees
hermes (Hermes (agent)) naps62-yolo (naps62-yolo (agent)) resende (Resende) naps62 (Miguel Palhas)
No Assignees
1 Participants
Due Date
No due date set.
Depends on
#167 Add a resume_on_boot policy for recovered sessions
naps62/maestro
Reference: naps62/maestro#175