fix(#502): make proxy-worker teardown deterministic — latch terminate onto in-flight shutdown, escalate on real exit evidence, verify the kill - #535
Merged
Conversation
… onto in-flight shutdown, escalate on real exit evidence, verify the kill Root cause: on natural debuggee termination the worker runs shutdown() as a floating promise off the DAP event; the parent's IPC terminate lands inside that ≥1s SHUTTING_DOWN window, handleTerminate early-returned, and the runner's post-command exit check (state === TERMINATED exactly) scheduled nothing — the worker completed its own shutdown and sat alive forever, reaped only by the 5s SIGKILL, and not even that on the paths where the escalation was neutered. Worker: shutdown() is re-entrant via a latched promise; terminate during SHUTTING_DOWN awaits it, so the exit is always scheduled. Init-failure rollback no longer clobbers a completed shutdown's TERMINATED. ProxyManager/adapter: stop()'s IPC-send, SIGKILL, and early-resolve guards key on exitCode/signalCode instead of .killed (which Node latches on any delivered signal); ProxyProcessAdapter.kill() likewise; worker pid retained across cleanup() and exposed as IProxyManager.getProxyPid(). SessionManager: terminal handlers retain their stop() promise (pendingProxyStop) and the worker pid; closeSession/stopProxyPreservingSession await the pending stop even after the reference is cleared; the status-driven 'exit' (worker claims dead — OS process may not be) now stops the proxy; a post-close liveness probe logs "leaked worker" with the pid on recurrence. HTTP: MCP_HTTP_STALE_SWEEP_INTERVAL_MS makes the 60s sweep testable; --log-file now reaches the CLI logger (attachSharedFileTransport), so the stale-session reap line is visible in the operator's log (sse: #533). Tests: worker latch unit test; stop()-escalation and getProxyPid unit tests; session-manager worker-reap suite; sweep-override unit tests; new e2e (mcp-server-smoke-http-stale-reap) driving the real server + StreamableHTTP client through abandon → reap → pid-dead with no force-kill — red on unfixed main, green in ~7s after. Case study: docs/case-studies/the-zombie-worker-and-the-frozen-force-kill.md (double self-attach: one session frozen in the parent's force-kill timer, a second attached to the zombie worker via SIGUSR1). Dogfooding also filed #533 and #534. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018KnLteX47RBLM3ZYF1jGbc
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018KnLteX47RBLM3ZYF1jGbc
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
isRunning() exit-evidence semantics; status-driven exit reap failure warn; stopProxyPreservingSession pending-stop await + failure paths; attachSharedFileTransport happy/idempotent/best-effort; handleHttpCommand --log-file wiring with a non-winston logger. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018KnLteX47RBLM3ZYF1jGbc
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.
Closes #502.
Root cause (sharper than the issue's audit)
The live mechanism is a race the worker loses against itself. On natural debuggee termination,
onTerminated/onCloserunshutdown()as a floating promise; state sits inSHUTTING_DOWNfor ≥1s (two 500ms grace waits + adapter tree-kill). The parent reacts to the same DAPterminatedevent withstop()→ IPCterminate, which lands inside that window (~7ms vs ~800ms):handleTerminateearly-returned, and the runner's sole exit scheduler (post-handleCommand,state === TERMINATEDexactly) scheduled nothing. The worker finished its own shutdown and sat alive forever in stateTERMINATED— reaped only by the 5s SIGKILL, and not even that on the paths where the escalation was neutered (.killedlatches on any delivered signal;cleanup()drops the handle before the escalation; the status-driven'exit'handler never stopped at all; no layer retained the worker pid).Reproduced deterministically on Linux, first try — worker log:
Fix
dap-proxy-worker.ts):shutdown()re-entrant via a latched promise;terminateduringSHUTTING_DOWNawaits the in-flight shutdown, sohandleCommandcompletes inTERMINATEDand the exit is always scheduled. Init-failure rollback no longer clobbers a completed shutdown's terminal state.stop()'s IPC-send, SIGKILL, and early-resolve guards key onexitCode/signalCode(actual death) instead of.killed;ProxyProcessAdapter.kill()likewise (win32 tree-kill preserved); worker pid retained acrosscleanup()and exposed asIProxyManager.getProxyPid();isRunning()de-poisoned.stop()promise (session.pendingProxyStop);closeSession/stopProxyPreservingSessionawait it even after the reference was cleared; the status-driven'exit'(expected !== undefined— worker claims dead, OS process may not be) now stops the proxy; a post-close, unref'd 1.5s liveness probe logsleaked worker (issue #502)with the pid so any recurrence logs itself.MCP_HTTP_STALE_SWEEP_INTERVAL_MSmakes the hard-coded 60s sweep testable;--log-filenow reaches the CLI logger (attachSharedFileTransport) so theReaping stale HTTP sessionline is visible in the operator's log (the sse path has the same defect — sse: --log-file never reaches the CLI logger, so sse-command lines (incl. stale-session reap warnings) are invisible #533).Tests
TERMINATED, one teardown, one status (tests/proxy/dap-proxy-worker.test.ts).killed=truebut the process never exited;getProxyPid()survives stop/cleanup; the pinned "resolves immediately when already exited" test now models death withexitCode, not.killed(semantics: Node latches.killedon signal send, not death).session-manager-worker-reap.test.ts— status-driven exit reaps (stopCalls === 1), real child exit doesn't (pinned behavior preserved),closeSessionawaits the in-flight stop, leak probe logs/stays silent by liveness.mcp-server-smoke-http-stale-reap.test.ts: real server + StreamableHTTP client (first use in the suite), mock session to natural termination, client abandoned without DELETE, worker pid captured via argv tags, polled to death, log asserted to contain the reap line and not the force-kill line. Red on unfixed main; green in ~7s with the fix (worker exits cleanly ~800ms after termination).Full unit (4260), integration, and lint green; pre-push gate passed.
Dogfooding
The bug was held open live with mcp-debugger on itself: one debug session frozen at a breakpoint inside the parent's force-kill timer (freezing the timer keeps the zombie alive indefinitely), and a second session attached to the zombie worker via
SIGUSR1to inspect it from the inside (idle event loop, IPC pipe only, argv-tagged to the reaped session). Write-up:docs/case-studies/the-zombie-worker-and-the-frozen-force-kill.md. Dogfooding also filed #533 (sse--log-filegap) and #534 (worker stderr forwarded at error level).🤖 Generated with Claude Code
https://claude.ai/code/session_018KnLteX47RBLM3ZYF1jGbc