Skip to content

fix(preview): stop leaking vspipe/ffmpeg when seeking or cancelling - #64

Merged
StuartCameronCode merged 1 commit into
mainfrom
fix/preview-orphans-process-group
Aug 8, 2026
Merged

fix(preview): stop leaking vspipe/ffmpeg when seeking or cancelling#64
StuartCameronCode merged 1 commit into
mainfrom
fix/preview-orphans-process-group

Conversation

@StuartCameronCode

Copy link
Copy Markdown
Owner

Seeking previews and cancelling jobs leave vspipe and ffmpeg running. Three
separate causes, all resting on the same accident.

Nothing ever killed those children deliberately. They tended to die because
their pipes closed with the worker and they took EPIPE at the next write. A child
blocked reading a slow source — a NAS share, in the report — writes nothing for
minutes, never notices, and keeps burning CPU. That also explains why this never
reproduced locally: with fast local I/O the cascade wins every time.

1. Preview mode had no cleanup whatsoever

main() returns at the --preview branch before ctrlc is installed, so
SIGTERM killed the worker outright without unwinding — Drop never ran. And
generate_preview holds vspipe/ffmpeg in locals, so
PipelineExecutor::terminate() could not have reached them even if it had run.

2. A seek could lose a preview entirely

PreviewGenerator tracked one _previewProcess, assigned after
await Process.start(...) returned. A seek arriving inside that window cancelled
whatever the field happened to hold, and the next assignment overwrote the
reference to the in-flight worker — untracked, never killed. Scrubbing cancels a
preview on every movement, so they accumulate. This is the reported symptom.

3. Cancel signalled the pid alone

Even a clean shutdown depended on the worker getting far enough to kill its own
children.

Fix

The worker makes itself a process-group leader (setpgid), so the app can
tear down the whole tree with one signal to -pid. ProcessTree does that, with
a fallback to pid-only signalling where groups are unavailable — which is exactly
today's behaviour, so this is never worse.

PreviewGenerator tracks every live preview in a set, registered at spawn, so
nothing can be lost in that window. Cancellation signals immediately and reaps in
the background — a seek must not wait out a shutdown grace, or scrubbing feels
broken. dispose() uses the waiting variant, since strays outliving the app are
worse than a pause.

Verified, not assumed

  • the worker really does become a group leader (pgrp 3362 → 3382)
  • Dart really does forward a negative pid to kill(2) (killPid(-pid)true)
  • the group signal kills a child that was SIGSTOPped first, so the result
    cannot be credited to the EPIPE cascade

Two tests: a process-group invariant test that stops the children before
signalling, and a rapid-seek test firing ten overlapping previews. Both would
pass vacuously without the SIGSTOP and the burst respectively — which is why they
are written that way.

Reported: seeking previews and cancelling jobs leave vspipe and ffmpeg running.
Three separate causes, all of which rely on the same accident.

Nothing ever killed those children deliberately. They tended to die because
their pipes closed with the worker and they took EPIPE at the next write -- but a
child blocked reading a slow source (a NAS share, in the report) writes nothing
for minutes, never notices, and keeps burning CPU on work nobody wants. That
also explains why this never reproduced locally: with fast local I/O the cascade
wins every time.

1. Preview mode never installed a signal handler at all -- main() returns at the
   --preview branch before ctrlc is set up -- so SIGTERM killed the worker
   outright without unwinding. Drop never ran. And generate_preview holds vspipe
   and ffmpeg in locals, so PipelineExecutor::terminate() could not have reached
   them even if it had run.

2. PreviewGenerator tracked one _previewProcess, assigned *after*
   `await Process.start(...)` returned. A seek arriving inside that window
   cancelled whatever the field happened to hold, and the next assignment then
   overwrote the reference to the in-flight worker -- untracked, never killed.
   Scrubbing cancels a preview on every movement, so those accumulate. This is
   the "bunch of processes" in the report.

3. cancel() signalled the worker's pid alone, so even a clean shutdown depended
   on the worker getting far enough to kill its own children.

The worker now makes itself a process-group leader (setpgid), so the app can
tear down the whole tree with one signal to -pid, and ProcessTree does that with
a fallback to pid-only signalling where groups are unavailable -- which is
exactly today's behaviour, so this is never worse. PreviewGenerator tracks every
live preview in a set, registered at spawn, so nothing can be lost in that
window. Cancellation signals immediately and reaps in the background: a seek must
not wait out a shutdown grace, or scrubbing feels broken. dispose() uses the
waiting variant, since strays outliving the app are worse than a pause.

Verified rather than assumed. The worker really does become a group leader
(pgrp 3362 -> 3382), Dart really does forward a negative pid to kill(2)
(killPid(-pid) returns true), and the group signal kills a child that was
SIGSTOPped first -- so it cannot be credited to the EPIPE cascade.

Tests: a process-group invariant test that stops the children before signalling,
and a rapid-seek test that fires ten overlapping previews and asserts nothing
survives. Both would pass vacuously without the SIGSTOP and the burst
respectively, which is why they are written that way.
@StuartCameronCode
StuartCameronCode merged commit cdc4816 into main Aug 8, 2026
4 checks passed
@StuartCameronCode
StuartCameronCode deleted the fix/preview-orphans-process-group branch August 8, 2026 14:51
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.

1 participant