Skip to content

fix(ios): reap orphaned simulator recorders with a graceful finalize window - #2457

Open
thymikee wants to merge 3 commits into
mainfrom
fix/ios-record-video-host-busy-2170
Open

fix(ios): reap orphaned simulator recorders with a graceful finalize window#2457
thymikee wants to merge 3 commits into
mainfrom
fix/ios-record-video-host-busy-2170

Conversation

@thymikee

@thymikee thymikee commented Sep 10, 2026

Copy link
Copy Markdown
Member

Summary

Narrows #2170 to its non-overlapping piece. #2447 already owns the simulator-recording start path — the exit-16 busy classifier, the fast-exit observation, and the graceful SIGINT rollback — under the DEVICE_IN_USE contract. To land a single start-path error contract, this PR keeps only the daemon-startup fix that #2447 does not touch.

When the daemon starts, it reaps orphaned simctl recordVideo children (purposes: ['simctl-screen-recording']). It previously used the generic 2s reaper grace. An orphaned recorder releases CoreSimulator's host-wide recording lock only after finalizing on SIGINT; force-killing it after 2s fixes the one device but leaves the lock dangling so every later recording on the host fails with EBUSY. This passes the recorder the same finalize grace the live stop path allows (5s). Bounded to the recorder purpose, so it never widens the web or generic reap budgets.

Adds one startup-wiring test proving the recording purpose is reaped with a >= 5000ms term timeout.

Closes #2170 (recorder-recovery half); start-path recovery and the busy error contract land in #2447.

Why 5s

  • simctl recordVideo finalizes the mp4 on SIGINT; the host recording slot frees only after that.
  • Orphan reaping is sequential, but only a live wedged recorder pays the grace (dead records clear
    instantly), and the host-wide lock permits at most one live recorder — so startup stays under the
    15s client timeout.

Validation

  • Added daemon-runtime-recording-reaper.test.ts: drives real startDaemonRuntime (reaper spied) and
    asserts it invokes reapOwnedProcessRecordsAtStartup for the simctl-screen-recording purpose with
    a graceful finalize window. Fails if the grace drops to the generic default.
  • pnpm check:affected --run, tsc, and oxlint pass on head 99a379f9f0.
  • Net change: src/daemon/server/daemon-runtime.ts +5 and one new test; host start path unchanged.
  • Scope note: the busy EBUSY failure path and in-session rollback are validated in fix(ios): recover simulator recorder startup failures #2447, not
    here; a normal start/stop run does not exercise this startup reap (it only matters after an
    ungraceful daemon exit leaves a recorder orphan).

Related: #2170, #2447.

Classify `simctl recordVideo` exit 16 (POSIX EBUSY) as a typed COMMAND_FAILED with
an actionable hint and the recorder's stderr instead of an unclassified UNKNOWN, so
a caller learns another recording holds the one host-wide CoreSimulator slot or a
prior recorder died without detaching.

Give a recorder that is rolled back or reaped the same graceful SIGINT -> SIGTERM ->
SIGKILL escalation the live stop path already uses, and widen the daemon-startup
reaper's recorder grace to match it, so finalize-and-detach releases the host-wide
recording lock instead of a mid-detach SIGKILL dangling it for every later recording.

Closes #2170
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.53 MB 4.53 MB +18 B
Package (unpacked) 4.53 MB 4.53 MB +18 B
Package (download) 1.34 MB 1.34 MB +12 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.0 ms 27.1 ms +0.2 ms
CLI --help 76.0 ms 75.7 ms -0.3 ms

…ack helper

Simplify #2170. The exit-code classification lives beside `startError` that is its only
caller, so drop the separate module and its test and assert through the real start path
instead. Test through `normalizeError` to keep the wire-shape (hint lifted, reason and
exit code retained) guarantee.

Revert the start-rollback SIGKILL sites: the identity arm fires only when the recorder
already exited, and the acquisition rollback discards an explicitly canceled recording, so
fast-kill there is fine. The dangling-host path is the startup reaper, which keeps the
wider finalize grace added earlier.
@thymikee

Copy link
Copy Markdown
Member Author

The startup failure is still missed at 1fb63cb: an immediate exit 16 can happen before process identity is readable, so waitForManagedProcessIdentity returns no marker and the new classifier is never reached. The test keeps identity metadata available for an already-exited child, masking that path. Observe process exit during identity acquisition and add a regression with missing identity plus exit 16.\n\nCanceled acquisition and identity-failure cleanup still use immediate SIGKILL. A recorder may already hold the stream slot there; discarding the video does not remove the need to detach cleanly. Reuse graceful rollback and verify recording works afterward. #2447 already addresses these same paths with a different error contract, so consolidate the overlapping changes before either lands.\n\nCI is green, but the reported normal start/stop run does not validate orphan recovery or the busy failure path.

…grace

The exit-16 classifier and graceful start rollback are owned by #2447 with the DEVICE_IN_USE contract; drop the overlapping COMMAND_FAILED classifier here to keep one start-path error contract. Keep only the non-overlapping daemon-startup fix: an orphaned simctl recorder is reaped with the same finalize grace the live stop path allows, plus a startup-wiring test. Refs #2170.
@thymikee thymikee changed the title fix(ios): hint the busy simctl recorder and detach recorders gracefully fix(ios): reap orphaned simulator recorders with a graceful finalize window Sep 10, 2026
@thymikee

Copy link
Copy Markdown
Member Author

@thymikee consolidating per your comment so #2447 and this don't land two start-path error contracts:

  • Fast exit before identity / busy classifier: you're right that at 1fb63cb an immediate exit 16 could settle before waitForManagedProcessIdentity reads a marker, so the classifier in startError was never reached, and the busy test kept identity metadata for an already-exited child and masked that path. Rather than duplicate the fix, I've dropped the exit-16 classifier and the graceful start rollback herefix(ios): recover simulator recorder startup failures #2447 already observes exits that settle before PID discovery and on the final identity poll, and does it with the DEVICE_IN_USE / apple_simulator_recording_busy contract. Point 1 and point 2 (fast-exit regression, graceful rollback) are fix(ios): recover simulator recorder startup failures #2447's to own and validate.
  • Consolidation: this PR is now reduced to the one non-overlapping piece fix(ios): recover simulator recorder startup failures #2447 does not touch — the daemon-startup reaper grace in daemon-runtime.ts. It reaps orphaned simctl recordVideo children with the recorder's 5s finalize window instead of the generic 2s, so the host-wide recording lock is released rather than left dangling. Net diff is now daemon-runtime.ts +5 and one test; the simulator-host start path is untouched.
  • Validation (point 3): the normal start/stop claim was for the reduced-scope change and did not cover orphan recovery or the busy path — corrected in the body. Orphan recovery is now covered by daemon-runtime-recording-reaper.test.ts (drives real startDaemonRuntime, asserts the recording purpose is reaped with a >=5000ms term timeout); the busy path and in-session rollback are validated in fix(ios): recover simulator recorder startup failures #2447's provider scenario.

Gate on head 99a379f9f0: check:affected, tsc, oxlint green.

@thymikee

Copy link
Copy Markdown
Member Author

The overlap is resolved at 99a379f: this now only gives the existing recorder-only startup reaper five seconds to finalize after SIGINT. No code findings. Before marking it ready, please show a CoreSimulator run that leaves a live recorder orphaned after an ungraceful daemon exit, restarts the daemon, and then successfully starts and stops a new recording. The wiring test and normal recording runs do not prove that recovery. The current iOS failure concerns alert replacement and appears unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS sim: ungraceful recorder kill leaves CoreSimulator host recording lock dangling (code 16 for all later recordings)

1 participant