interleaver: don't let a parked worker or an early stop escape cleanup - #720
Merged
Merged
Conversation
…p it `envoy.interleave` calls `check_dangling_mediators()` only on the success path. When the model's forward raises it goes straight to `finally: cancel()`, which set `mediator.worker = None` — but dropping a reference does not end a greenlet. The worker stays parked, keeping its frame, the frame keeps the block's `Scope`, and the `Scope` keeps the model. Not even a gc pass reaches it: a suspended greenlet's frames are invisible to the cyclic collector. Measured at about 0.5 GiB of CUDA per errored trace, cumulative for the life of the process. So throw `GreenletExit` into any worker still `alive`, which unwinds it and runs the block's `finally` blocks. An exception raised out of one of those warns rather than propagating: cancel runs in the driver's own `finally`, with the error that ended the run already in flight, and that error is the one worth surfacing. `check_dangling_mediators` sets the precedent for telling the two apart, and is deliberately still not called on the error path — it raises `OutOfOrderError`, which would bury the real error. `cancel()` is the one choke point for `envoy.interleave`, `tracer.py` and the vLLM tracer. On the vLLM client side the workers are serialized onto requests and never started, so `alive` is False there and nothing changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`EarlyStopException` subclasses `Exception` and `defer_exceptions` is False on a local trace, so the raise in the mediator loop left `Interleaver.handle` immediately — skipping the rest of `ready`, the count update, `assemble_skip`, the `observers` loop that feeds `tracer.cache`, and the fragment `undo` (the TP re-split). Two symptoms from that one cause: a stop at layer 5 left layer 5 out of the cache, and a stop in one invoke of a batched generate cost every sibling invoke that step, so the invokes came back with different step counts. An early stop is control flow, not an error: the worker asked to halt what comes *after* the location it is parked on, not to abandon that location. So catch it, keep it in a local, clear that mediator's `pending`, finish the visit, and re-raise just before the return. Other exceptions keep today's behaviour, and `defer_exceptions` still records a stop on its mediator rather than raising, so the vLLM path is unchanged. The loop still terminates: a mediator with no pending is not ready. Three consequences, now stated in docs/usage/stop-and-early-exit.md rather than left implicit: the module you stop at is still recorded and served; the workers parked on the same visit are still served; and a stop in one invoke ends the whole batched forward, since it is one shared run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Two findings from the agent stress sweep, items 2 and 3 in
planning/next-steps.md. Same shape of problem — something escaping before the cleanup around it finishes — but independent fixes, so one commit each.1.
cancel()leaked a parked workerBroken:
envoy.interleavecallscheck_dangling_mediators()only on the success path; when the model's forward raises it goes straight tofinally: cancel(), which setmediator.worker = Nonewithout unwinding. Dropping a reference does not end a greenlet, so the worker stayed parked, holding its frame, the block'sScope, and through it the model. A gc pass does not reach it either — a suspended greenlet's frames are invisible to the cyclic collector. Roughly 0.5 GiB of CUDA held per errored trace, cumulative (repro:tasks/skip-stop-scan-corners/noskills/repro_8.py).Changed:
cancel()throwsGreenletExitinto any worker stillalive, which unwinds it and runs the block'sfinallyblocks. An exception out of one of those warns rather than propagating, so it cannot mask the model error already in flight (check_dangling_mediatorsis the precedent for telling the two apart).check_dangling_mediatorsis deliberately still not called on the error path — it raisesOutOfOrderError, which would bury the real error.cancel()is the single choke point forenvoy.interleave,tracer.pyand the vLLM tracer. On the vLLM client side the workers are serialized onto requests and never started, soaliveis False there and nothing changes.2.
handle()abandoned the visit ontracer.stop()Broken:
EarlyStopExceptionsubclassesExceptionanddefer_exceptionsis False on a local trace, so the raise in the mediator loop leftInterleaver.handleimmediately — skipping the rest ofready, the count update,assemble_skip, theobserversloop that feedstracer.cache, and the fragmentundo. Two symptoms from one cause:model.transformer.h.5out of the cache;len(picks) == 3butlen(sibling) == 2.Changed: an early stop is treated as control flow — caught, kept in a local, that mediator's
pendingcleared, the visit finished, and the exception re-raised just before the return (afterundo). Other exceptions keep today's behaviour, anddefer_exceptionsstill records a stop on its mediator rather than raising it, so the vLLM path is unchanged. The loop terminates because a mediator with no pending is not ready.The three deliberate calls are now stated in
docs/usage/stop-and-early-exit.md(two new sections plus two gotcha bullets): the module you stop at is still recorded and served; the workers parked on the same visit are still served; and a stop in one invoke ends the whole batched forward, since it is one shared run.docs/developing/interleaver-internals.mdhad two sentences describing the old behaviour ofcanceland of stop, so both were corrected — the only file I touched outside my assignment, and it is the developer page for the function I changed.Tested
Environment:
nnsight-stress/envwithPYTHONPATHat this worktree'ssrc. The optional_c/py_mountextension is not built there, so.save()as a method fails and about half the suite errors; I compiled it into the worktree (gitignored) to get a real run.New tests, all five verified failing against
origin/0.8and passing here:tests/test_interleaving.py—cancelunwinds a still-parked worker (itsfinallyruns);cancelwarns rather than raises when the unwind hits a userfinally; an early stop finishes the visit (the sibling worker is served and the count is bumped) before it raises.tests/test_memory.py— a model that raises mid-forward frees the module afterwards; five such traces do not accumulate. CPU-only, adapted fromrepro_8.py, which needed CUDA.tests/test_language.py::TestEarlyStop— the module stopped at is still in the cache; a stop in one invoke leaves both invokes with the same step count.Ran clean, per file:
test_interleaving,test_memory,test_language,test_batching,test_tracing,test_saving,test_editing,test_backward,test_envoy,test_source,test_fragments,test_modeling,test_vlm,test_diffusion. Both new doc snippets were run verbatim and produce what the page claims. (Running several test files in one pytest invocation errors ontransformers.pipelinefor unrelated, pre-existing reasons, as doestest_serializationon its own — both reproduce identically onorigin/0.8.)Not tested / left alone
tests/vllm/could not run. Thedefer_exceptionsbranch is unchanged by construction: a stop under deferral is still recorded on its mediator and never raised out of the handoff.undopath (tensor parallel) is picked up by the stop fix —undonow runs before the re-raise — but there is no multi-device setup here to exercise it. Per the item note, it is not skipped, just untested.check_dangling_mediatorscall was added to the error path, per the decision note.🤖 Generated with Claude Code