Repository navigation
fix(win): close the pseudoconsole when the shell exits on its own - #991
Open
Roi Dayan (roidayan1) wants to merge 1 commit into
Open
Roi Dayan (roidayan1) wants to merge 1 commit into
Roi Dayan (roidayan1) wants to merge 1 commit into
Conversation
Fixes microsoft#965. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Greg Priday (gregpriday)
added a commit
to daintreehq/daintree
that referenced
this pull request
Oct 13, 2026
- Patch node-pty's conpty.cc in postinstall with the fix proposed in microsoft/node-pty#991: the exit watcher no longer drops the pty record without ClosePseudoConsole, which left one conhost.exe per self-exited shell running for the life of the pty-host (microsoft/node-pty#965) - kill() closes the pseudoconsole at most once; a second call on a live pty corrupted the heap (0xC0000374) in stock 1.2.0-beta.14 - Anchored edits fail the install when node-pty's source changes, and CI treats that failure as deterministic rather than retrying it
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 #965.
When a shell exits on its own, the exit watcher in
src/win/conpty.ccerases the pty's batonbefore
onExitreaches JS, and nothing callsClosePseudoConsole.kill()can't reach it oncethe baton is gone, so every pty leaves its
conhost.exe, and the pseudoconsole's handles, untilthe host process exits.
hpcfrom the baton underg_ptyHandlesMutex, as it erases the baton.Nothing else holds it once the baton is erased.
hpcto a short-lived thread that closesit. So the exit code is in JS before the output pipe can close, and the main thread never waits
on the close (see the last paragraph).
PtyConnect, whereuseConptyDllis known.PtyKillnow nullshpconce it has closed it, and skips a null one. That's the same hunk asfix(win): don't close the pseudoconsole twice on a second kill() #989. The watcher relies on it, so that a
kill()that wins the race isn't followed by a secondclose. Whichever PR lands first, the other rebases onto an identical change.
useConptyDll: true, OpenConsole.exe already exits on its own, but its HPCON stayedopen. This closes it too.
One visible change: with the default ConPTY,
exitnow arrives when the output pipe closes,about 60 ms after the shell exits, rather than after
_flushDataAndCleanUp's 1 s timer. Forcmd.exe /c exit 3it came at 92–101 ms after spawn, against 1,044–1,051 ms on 1.2.0-beta.15,with the exit code 3 every time. A paused pty is unchanged:
exitstill comes from the 1 stimer's cleanup.
The #763 test let
cmd.exe /c echo testexit by itself and attached itsexitlistener 100 msafter
ready_datapipe, so it relied on that delay. It now runs an interactivecmd.exethat waitsfor its
kill(), which keeps what it checks: the pid afterready_datapipe.New tests in
windowsTerminal.test.ts, run in both ConPTY modes:conhost.exe or OpenConsole.exe is gone within 5 s of the exit. It fails on
mainwith thedefault ConPTY.
0–90 ms unless its exit has arrived. Without the
PtyKillhunk, the test process dies with0xC0000374.
Checked on Windows 11 Pro build 26300 (x64), with Node 24.19.0 and VS 2022 Build Tools:
windowsTerminaltests, with the new onesmain: 27 of 28 pass; the #965 one fails with the default ConPTYnpm test(every test, then eslint)mainand 1.2.0-beta.15: 5PtyKillhunkThe remaining 9 handles per pty are outside this change.
Why the close has a thread of its own: before Windows 11 24H2,
ClosePseudoConsolewaitsuntil the output pipe is drained (per its docs). The conout worker drains it only as fast as the
main thread reads: it
pipe()s the pipe into the socket the main thread reads. So a paused orslow pty would leave a close on the main thread waiting for a read that can't come. Build 26300
returns from the close at once, so that wait couldn't be reproduced here. On it, a paused pty
whose shell printed and exited kept the main thread running and left no host.
PtyKill's closestays where it was.
The thread runs
conpty.node's own code, so the first close pins the module(
GetModuleHandleExWwithGET_MODULE_HANDLE_EX_FLAG_PIN, once). Node unloads a Worker's addonswhen the Worker ends, and the close could still be running.
This change was developed with the help of an AI coding assistant, then built and tested on Windows by me as described above.