fix: death-aware, retryable js-debug child adoption; remove dead configDone deferral - #251
Merged
Merged
Conversation
…250) The 1500ms parent-configurationDone deferral could never arm on any real path: its only writer runs after createChildSession resolves, but the js-debug handshake sends parent configurationDone before launch/attach, so no child session can exist yet. Even when force-armed (tests only), the deferral never settled early - nothing ever called its resolve; it always burned the full 1500ms timer. Also removes the equally dead AdapterPolicy.shouldDeferParentConfigDone() (no src consumer) and the DapClientBehavior.deferParentConfigDone flag from all eight policies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A child target dying mid-adoption used to burn every remaining step's full timeout window (worst case ~400s in the attach retry loop alone) and left the half-adopted socket orphaned at worker shutdown. - New settle-once ChildDeathLatch (modeled on JsDebugLaunchBarrier) subscribes close/error right after connect and aborts the in-flight adoption step the moment the child dies. - attachChild gains a 60s total deadline on top of per-request timeouts and checks the latch before and after each retry. - waitForEvent settles immediately on client close/error instead of waiting out its timer against a dead client. - The failure path now rolls back childSessions/activeChild (previously hasActiveChildren() stayed true forever, silently blocking every subsequent adoption) and shuts down the failed child client. - MinimalDapClient.shutdown() now also invokes ChildSessionManager shutdown, reaching mid-adoption sockets that were never promoted via childCreated; ensureChildStopped's pause calls get explicit 5s timeouts (were 30s defaults). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The js-debug reverse startDebugging handler adds the pending target id
to the client's adoptedTargets set before adoption runs, but nothing
removed it when createChildSession rejected - so a re-sent
startDebugging for the same target was answered {handled: true} without
ever creating a child, leaving the session silently undebugable.
The dispatch site (which already observes the rejection) now deletes
the id, resyncing the policy-visible set with ChildSessionManager's own
rollback; shutdown() additionally clears the set so a reused client
cannot carry stale adoption state.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Fixes #248
Fixes #249
Fixes #250
First of two PRs for the
cluster:js-child-sessionbacklog chunk (#247 follows separately).What changed
#250 — remove dead
deferParentConfigDonemachinery (commit 1). The 1500ms parent-configurationDonedeferral could never arm on any real path: its only writer runs aftercreateChildSessionresolves, but the js-debug handshake sends parentconfigurationDone(step 4) beforelaunch/attach(step 5), so no child can exist when the parent configDone goes out. Even force-armed, nothing ever resolved the deferral early — it always burned the full timer. Deleted along with the equally deadAdapterPolicy.shouldDeferParentConfigDone()and theDapClientBehavior.deferParentConfigDoneflag from all eight policies (−233 lines).#248 — death-aware adoption (commit 2). A child target dying mid-adoption used to burn every remaining step's full timeout (worst case ~400s in the attach retry loop;
ensureChildStoppedalone could add 80s) and left the half-adopted socket orphaned at worker shutdown:ChildDeathLatch(modeled on fix: js-debug launch barrier settles on debuggee death (fixes #242) #246'sJsDebugLaunchBarrier) subscribesclose/errorright after connect; every adoption step is raced against it and aborts the moment the child dies.attachChildgains a 60s total deadline on top of per-request timeouts, with death checks before/after each retry.waitForEventsettles immediately on client death instead of waiting out its timer.childSessions/activeChild— previouslyhasActiveChildren()latched true forever, silently rejecting every subsequent adoption — and shuts down the failed child client.MinimalDapClient.shutdown()now also invokesChildSessionManager.shutdown()(previously unreachable from src), covering mid-adoption sockets never promoted viachildCreated.#249 — retryable adoption (commit 3). The policy-level
adoptedTargetsdedupe kept the pending-target id when adoption failed, so js-debug's re-sentstartDebuggingwas answered{handled: true}without creating a child. The dispatch site (which already observes the rejection) now rolls the id back, andshutdown()clears the set.Testing
startDebuggingretries,adoptedTargetscleared on shutdown. The death tests run in ~35ms where the old behavior hung 15s+ (test-timeout kills).🤖 Generated with Claude Code