Skip to content

Demucs worker: teardown skipped on exception, and cancel is dropped during the GPU to CPU fallback #514

Description

@thcp

Two verified defects in the Demucs separation path, both in app/pipeline/separate.py.

1. Persistent-worker teardown sits outside the try/finally

separate.py:202-212:

    finally:
        _done_evt.set()
        set_proc(job.id, None)
        wt.join(timeout=2)

    # Never reuse a worker after anything but a clean success ...
    if job_ok is not True:
        _kill_worker()

The finally ends at line 205. The teardown at 211-212 is outside it.

If anything in the read loop raises -- proc.stderr.read(1) hitting OSError when the API thread's terminate() races the read, or _set() raising -- the exception propagates out of _run_demucs and _kill_worker() never executes. _worker["proc"] still holds the process, so the next job's _get_worker(device) sees a matching device and a live poll() and reuses a worker whose CUDA/MPS state followed an exception.

This is exactly what .claude/rules/ml-pipeline.md forbids: "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."

Fix: move the teardown inside the finally.

2. Cancellation is dropped during the GPU to CPU fallback

Verified: _run_demucs has no cancel_requested check at entry -- it goes straight to _get_worker(device) then set_proc(job.id, proc). And separate() has no check between the two attempts (separate.py:241-260).

Scenario:

  1. The GPU attempt fails. _run_demucs returns rc=1, having already called set_proc(job.id, None) in its finally.
  2. The user presses cancel during shutil.rmtree(job_dir / DEMUCS_MODEL) (line 255 -- seconds for a multi-GB partial) or during _get_worker("cpu")'s spawn and model load.
  3. cancel_job finds registry_get_proc(job_id) is None and terminates nothing.
  4. _run_demucs(job, source, job_dir, "cpu") runs the entire CPU separation to completion -- 10+ minutes -- and only then does line 217 raise JobCancelled.

The UI shows "Cancelling" for the whole run.

Fix: check cancel_requested at the top of _run_demucs and again before dispatching the CPU fallback.

Test

Per .claude/rules/testing.md, every pipeline stage needs a cancellation test and a subprocess-failure test. Both cases above are currently uncovered.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingreliabilityStability, resource management, race conditions

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions