windows: avoid conPTY deadlock when debugger pauses conout worker - #943
Merged
Conversation
The conPTY output connection is established on a worker thread before the
main Node.js thread calls the native connect implementation. Calling
conptyNative.connect() before that worker reports readiness is unsafe because
the native implementation synchronously calls ConnectNamedPipe() for the
input and output pipes.
ETW stacks from the frozen Code - OSS agent host showed:
agentHostTerminalManager._spawnPty
-> node-pty.spawn
-> WindowsTerminal
-> WindowsPtyAgent
-> ConoutConnection
-> Worker
The conout worker was stopped in the Node inspector startup message loop while
processing Debugger.enable. Five seconds later, the agent-host event loop
thread entered the WindowsPtyAgent timeout fallback and proceeded through
conptyNative.connect() into NtFsControlFile/ConnectNamedPipe. Since the paused
worker had not connected the output side, ConnectNamedPipe waited
synchronously and blocked the event loop. Consequently, CDP's Runtime.enable
request could not complete and the debugger attachment appeared frozen.
The worker-ready handshake was originally introduced to prevent this
deadlock, but a later timeout fallback called connect() anyway to avoid
leaving the PTY in a zombie state. That fallback violated the handshake
invariant and restored the blocking path under debugger induced worker
delays.
Make worker readiness a hard prerequisite for calling connect():
- Fail and clean up the pending PTY when the readiness watchdog expires
instead of attempting the native connection.
- Propagate worker startup errors and premature exits to WindowsPtyAgent.
- Kill the pending native PTY, dispose the worker, destroy its sockets, and
report the failure through onError.
- Clear the watchdog after readiness, connection failure, or explicit kill.
- Ignore readiness and error events arriving after timeout or termination.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dmitrivMS
approved these changes
Jul 31, 2026
deepak1556
enabled auto-merge (squash)
July 31, 2026 18:27
connor4312
approved these changes
Jul 31, 2026
asiloisad
added a commit
to lumine-code/node-pty
that referenced
this pull request
Aug 4, 2026
Picks up the conPTY deadlock fix (microsoft#943), which avoids hanging when a debugger pauses the conout worker. That is the only upstream change the fork was missing; the tar devDependency bump in the same range is already superseded here. The one conflict was in windowsPtyAgent.ts, where upstream added a _connectionTimeout field next to the _closeTimeout whose type this fork had corrected from NodeJS.Timer to NodeJS.Timeout. Kept both. Upstream's new specs used mocha's this.timeout(), which throws under the jasmine runner this fork moved to; dropped the call, since the helper in spec/support/timeout.js already raises the ceiling globally. Note that beta.15 still calls Napi::Error::Fatal from SetupExitCallback in both src/unix/pty.cc and src/win/conpty.cc, still ships no NAPI defines in binding.gyp, and is still on node-addon-api 7 — so the teardown fix this fork exists for remains ours alone.
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.
The conPTY output connection is established on a worker thread before the main Node.js thread calls the native connect implementation. Calling conptyNative.connect() before that worker reports readiness is unsafe because the native implementation synchronously calls ConnectNamedPipe() for the input and output pipes.
From ETW trace reported by @connor4312, the frozen
Code - OSSagent host showed:The conout worker was stopped in the Node inspector startup message loop while processing Debugger.enable. Five seconds later, the agent-host event loop thread entered the WindowsPtyAgent timeout fallback and proceeded through conptyNative.connect() into NtFsControlFile/ConnectNamedPipe. Since the paused worker had not connected the output side, ConnectNamedPipe waited synchronously and blocked the event loop. Consequently, CDP's Runtime.enable request could not complete and the debugger attachment appeared frozen.
The worker-ready handshake was originally introduced to prevent this deadlock in #885, but a later timeout fallback called connect() anyway to avoid leaving the PTY in a zombie state. That fallback violated the handshake invariant and restored the blocking path under debugger induced worker delays.
Make worker readiness a hard prerequisite for calling connect():