Conversation
thcp
marked this pull request as ready for review
May 4, 2026 09:59
thcp
added a commit
that referenced
this pull request
Aug 31, 2026
… before the CPU retry (#534) Fixes #514. Independent; branches off `0.16.1`. ## 1. Worker teardown sat outside the `finally` ```python finally: _done_evt.set() set_proc(job.id, None) wt.join(timeout=2) if job_ok is not True: # <- outside _kill_worker() ``` An exception out of the read loop -- `proc.stderr.read(1)` raising `OSError` when the API thread's `terminate()` races the read, or `_set()` raising -- propagated without it. `_worker` still held the process, so the next job's `_get_worker()` saw a matching device and a live `poll()` and **reused a worker whose CUDA state followed an exception**. `.claude/rules/ml-pipeline.md`: *"torn down after **any** non-success (cancellation or failure) -- post-exception CUDA state can't be trusted. Both halves of this rule matter; don't relax either side."* ## 2. A second path missed teardown entirely The pipe check raises **before** the `try`: ```python if proc.stdin is None or proc.stderr is None: raise RuntimeError("demucs worker has no stdin/stderr pipe") ``` so a worker that came back without pipes stayed cached and would be handed to every subsequent job. Found while writing the test for #1 -- it failed for this reason rather than the one I was targeting. ## 3. Cancel was dropped before the CPU retry `separate()` had no cancel check between attempts. The `rmtree` of a multi-GB partial takes seconds with nothing registered for cancel, so a cancel landing there was invisible and the **entire CPU pass ran to completion** -- 10+ minutes -- before `JobCancelled` was raised at the end. The UI showed "Cancelling" throughout. ## 4. `_kill_worker` killed without reaping `kill()` only sends the signal. Without `communicate()` a worker wedged in an uninterruptible CUDA call becomes a zombie whose pipes close only when the `Popen` refcount happens to drop. `vocal_split.py` already pairs the two. ## A check I tried and removed I initially added a cancel check at the top of `_run_demucs`. It broke `test_cancel_kills_worker_next_job_spawns_fresh`, which verifies that a cancelled job tears the worker down so the next spawns fresh -- a check that never spawns has no worker to tear down, so the test's premise collapses. That test encodes exactly the rule this PR is protecting, so the check went rather than the test. The queue worker already gates dispatch on cancellation, and the fallback check covers the window that was actually reported, so nothing is lost. ## Verification New `tests/test_separate_worker_lifecycle.py`, 3 tests. Confirmed not vacuous -- reverting the teardown placement and the fallback check fails them. ``` ruff check All checks passed pytest tests/ 896 passed, 2 failed (pre-existing ogg pair) ``` `tests/test_separate_fallback.py` (13 tests) still passes unchanged -- worth noting, since that is the suite that pushed back on the entry-point check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Initial import of StemDeck.
Local source-separation tool: Demucs htdemucs_6s backend, FastAPI + SSE, vanilla-JS DAW-style frontend. macOS/Linux setup via run.sh, Docker option in build/.