Repository navigation
fix(proxy): spawn debug adapters detached on POSIX only; on win32 the missing console made every Delve launch 8-13 s slow (#889) - #931
Merged
Conversation
… missing console made every Delve launch 8-13 s slow (#889) Node's `detached` is DETACHED_PROCESS on Windows: the adapter gets no console. Delve pays for that at every process it creates. Measured on Windows 11, idle: `dlv version` takes 0.1 s with an inherited or hidden console (CREATE_NO_WINDOW, which windowsHide sets) and 2-3 s under DETACHED_PROCESS or CREATE_NEW_CONSOLE, isolated per creation flag; go, node, python, java, ruby and CodeLLDB start in the same time either way. Under a detached `dlv dap`, Delve's own log shows the warm `go build` taking 6 s (0.2 s in a shell) and the debuggee start 4 s, so a 1.16 s launch took 8-13 s on the main path and crossed the two fixed 30 s launch limits under pre-push load, which is the #889 flake: all four sightings' proxy logs show `launch`, Delve's `Building ...` output event 2 ms later, then silence until the kill. The cold build the issue blamed is 3-9 s with a throwaway GOCACHE; a beforeAll warm build was measured and dropped (it saved ~2 s and left the stall). The POSIX process group is what shutdown() signals (-pid, issue #337), so detached stays there; the win32 tree-kill is taskkill /T and never used a group. Two unit tests pin the per-platform flag. After: the Go integration suite alone 48 s -> 15 s, each debug-mode case 5.7 s, launch -> response 1.17 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PKWRJTTqYdYrhDn2z9Et5P
…d now say unref'd, detached off win32 (#889) Five comments taught the pre-#889 mechanism: the python adapter's pythonw rationale (the launcher got a visible console because the adapter had none; it now inherits the adapter's hidden one), the reaper, janitor and js adapter notes about "detached+unref'd" vsDebugServer instances (the stranding comes from Windows having no parent-death signal, not from detached), the worker launcher's "detached adapter children", and the adapter manager's own spawn log comment. The spawn comment also names the condition CREATE_NO_WINDOW depends on: no stdio slot may be 'inherit'. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PKWRJTTqYdYrhDn2z9Et5P
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 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.
Closes #889.
What the measurement found
The issue blamed Delve's cold
go buildinside the 30 s proxy-init deadline. The build is a minority of the time. Reproduced on this Windows 11 box, idle:dlv dapwas startedlaunch→ responsespawn(...)plain,windowsHide: truespawn(...)withdetached: true(the adapter manager, before this PR)Delve's own log under the detached spawn (
--log --log-output=dap,debugger): thego buildthat takes 0.2 s warm in a shell took 6 s, andlaunching process→Adding target(starting the debuggee) took 4 s. Isolating the Windows creation flags with Python'screationflagsondlv version:DETACHED_PROCESS(what Node'sdetachedsets, no inherited console) andCREATE_NEW_CONSOLEtake 2–3 s each;CREATE_NO_WINDOW(whatwindowsHidesets) andCREATE_NEW_PROCESS_GROUPtake 0.1 s like a plain spawn.go version,node,python,java,rubyand CodeLLDB start in the same time detached or not — only Delve pays, and it pays once per process it creates.So a debug-mode launch of a trivial program was 8–13 s idle on the main path (measured through the dev backend too: 5 s to accept the connection, 10 s to answer
launch), and under pre-push load it crossed the two fixed 30 s limits thatlaunchsits under: the proxy's initialization deadline (DEFAULT_PROXY_INITIALIZATION_TIMEOUT_MS) and the worker'sMinimalDapClientrequest timeout. The four sightings' proxy logs all have the same shape:launchsent, Delve'sBuilding …output event 2 ms later, then silence until the kill.The cold build itself: 3–9 s with a throwaway
GOCACHE, 0.2 s warm. AbeforeAllwarm build (the issue's proposal 1) was written, measured, and dropped: it saved ~2 s and left the 5–10 s stall in place.The fix
GenericAdapterManager.spawnpassesdetached: this.platform !== 'win32'. On POSIXdetachedputs the adapter in its own process group, whichshutdown()'s group kill relies on (#337); on win32 the tree-kill istaskkill /Tand never used the group, andwindowsHidekeeps the adapter window-less (CREATE_NO_WINDOW). Two unit tests pin the per-platform flag.Measured after
tests/adapters/go/integration/go-exit-code.test.tsalone: 48 s → 15 s; the two debug-mode cases 15–20 s → 5.7 s each; the proxy log'slaunch→ response 1.17 s (debug) / 0.23 s (noDebug).stopOnEntry, continue, exit 0), JavaScript (exit_code_test.js, exit 7), Go (exit_code, exit 3) all paused at entry and ran to completion; nodlv.exe, debugpy or js-debug process left afterclose_debug_session.--project e2e: javascript-attach (10), cpp-attach (2), go (7), python (5) — 24/24; then dotnet (3), java (7), ruby (6), rust (4), dart (8) — 28/28; cobol (28) — 28/28. Every adapter's spawn changed on Windows, and every adapter's smoke ran.Verification
npm run typecheck:all,npm run lint,pnpm run changelog:checktests/unit/proxy,tests/proxy,session-manager-operations-coverage(1163 tests)dist/🤖 Generated with Claude Code
https://claude.ai/code/session_01PKWRJTTqYdYrhDn2z9Et5P