Skip to content

fix(win): close the pseudoconsole when the shell exits on its own - #991

Open
Roi Dayan (roidayan1) wants to merge 1 commit into
microsoft:mainfrom
roidayan1:fix-965-close-pseudoconsole
Open

Roi Dayan (roidayan1) wants to merge 1 commit into
microsoft:mainfrom
roidayan1:fix-965-close-pseudoconsole

Conversation

@roidayan1

Copy link
Copy Markdown

Fixes #965.

When a shell exits on its own, the exit watcher in src/win/conpty.cc erases the pty's baton
before onExit reaches JS, and nothing calls ClosePseudoConsole. kill() can't reach it once
the baton is gone, so every pty leaves its conhost.exe, and the pseudoconsole's handles, until
the host process exits.

  • The watcher now takes hpc from the baton under g_ptyHandlesMutex, as it erases the baton.
    Nothing else holds it once the baton is erased.
  • The exit callback passes JS the exit code, then hands hpc to a short-lived thread that closes
    it. 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).
  • The close function is resolved in PtyConnect, where useConptyDll is known.
  • PtyKill now nulls hpc once it has closed it, and skips a null one. That's the same hunk as
    fix(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 second
    close. Whichever PR lands first, the other rebases onto an identical change.
  • With useConptyDll: true, OpenConsole.exe already exits on its own, but its HPCON stayed
    open. This closes it too.

One visible change: with the default ConPTY, exit now arrives when the output pipe closes,
about 60 ms after the shell exits, rather than after _flushDataAndCleanUp's 1 s timer. For
cmd.exe /c exit 3 it 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: exit still comes from the 1 s
timer's cleanup.

The #763 test let cmd.exe /c echo test exit by itself and attached its exit listener 100 ms
after ready_datapipe, so it relied on that delay. It now runs an interactive cmd.exe that waits
for its kill(), which keeps what it checks: the pid after ready_datapipe.

New tests in windowsTerminal.test.ts, run in both ConPTY modes:

Checked on Windows 11 Pro build 26300 (x64), with Node 24.19.0 and VS 2022 Build Tools:

Before This PR
windowsTerminal tests, with the new ones main: 27 of 28 pass; the #965 one fails with the default ConPTY 28 of 28
npm test (every test, then eslint) 61 passing, lint clean
conhost.exe children 2 s after five shells exited on their own main and 1.2.0-beta.15: 5 0
handles kept per pty after 20 natural exits, default / conpty.dll 1.2.0-beta.15: 14 / 11 9 / 9
40 shells per mode, each killed twice at 0–39 ms unless its exit had arrived process alive, no host left
a paused pty whose shell printed 90 KB and exited 1.2.0-beta.15: a conhost.exe left the main thread ran on (longest gap between 50 ms ticks: 63 ms), no host left
without the PtyKill hunk the test run dies with 0xC0000374

The remaining 9 handles per pty are outside this change.

Why the close has a thread of its own: before Windows 11 24H2, ClosePseudoConsole waits
until 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 or
slow 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 close
stays where it was.

The thread runs conpty.node's own code, so the first close pins the module
(GetModuleHandleExW with GET_MODULE_HANDLE_EX_FLAG_PIN, once). Node unloads a Worker's addons
when 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.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ConPTY: the pseudoconsole is never closed on a natural shell exit — the exit watcher erases the baton before onExit, leaking one conhost.exe per pty

1 participant