Skip to content

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

Open
Tyler Vorpahl (Tyler-V) wants to merge 1 commit into
microsoft:mainfrom
Tyler-V:fix-close-pseudoconsole-on-exit
Open

Tyler Vorpahl (Tyler-V) wants to merge 1 commit into
microsoft:mainfrom
Tyler-V:fix-close-pseudoconsole-on-exit

Conversation

@Tyler-V

Copy link
Copy Markdown

Fixes #965.

When the shell exits on its own, the exit watcher in SetupExitCallback erases the pty baton without closing its pseudoconsole. kill() is the only other path to ClosePseudoConsole, and it does nothing once the baton is gone. So every pty whose shell exits by itself leaves a conhost.exe --headless running until the host process exits. Any child still attached to that console keeps running too, because it never gets CTRL_CLOSE_EVENT.

The fix:

  • PtyConnect resolves ClosePseudoConsole (or ConptyClosePseudoConsole with useConptyDll) the same way PtyKill does and passes it to the watcher. The watcher thread can't call LoadConptyDll because that throws N-API errors.
  • The watcher copies hpc under the lock, erases the baton, queues the exit code with BlockingCall, and then closes the pseudoconsole outside the lock. Closing after BlockingCall puts the exit code in the queue ahead of the output pipe closing, which is what fires exit. Closing before Release() means a close that blocks on Windows before 24H2 never stalls the finalizer's join() on the main thread. If BlockingCall returns napi_closing, the close is skipped because the host process is going away.
  • After PtyKill closes the pseudoconsole it sets hpc to null and skips the close when hpc is already null. Without this, kill-then-exit would close it twice. This hunk matches fix(win): don't close the pseudoconsole twice on a second kill() #989 line for line on purpose, so the two PRs merge in either order.

How I verified it (Windows 11 build 26200, Node 24.15.0, on top of d182c60):

  • Six ptys of cmd.exe /d /c exit 0. After every onExit plus 3 s, I counted the process's direct conhost.exe children. Before the fix: +6, all --headless. After: +0. With useConptyDll: true, both builds show +0 hosts.
  • cmd.exe /d /k running ping -n 300 127.0.0.1 in the foreground, then I terminated only the shell. Before the fix, ping and conhost were still alive 3 s after onExit, and onExit took about 11 s to fire. After: both were gone and onExit fired in 66 ms. With useConptyDll: true, before the fix onExit did not fire at all while ping was attached (I waited over 2 minutes). After the fix it fired in 72 ms and ping was gone.
  • I ran five rounds each of kill before exit, kill twice, and kill after exit, with and without useConptyDll. No crashes and no leftover hosts.
  • 100 natural exits with varying exit codes (30 with useConptyDll) all reported the right code.

I added a test in windowsTerminal.test.ts that runs for both useConptyDll values. It spawns cmd.exe /C exit and checks that no new conhost or OpenConsole child of the test process outlives exit. Against the unfixed build it fails for useConptyDll = false. The useConptyDll = true case passes before and after and is there as a guard. npm test passes: 59 tests plus lint.

Two caveats:

  • I could only test on build 26200, where ClosePseudoConsole returns immediately. Before 24H2 it waits until the console host exits, which needs the output pipe drained. Here that wait runs on the watcher thread after the exit code is queued, while the conout worker keeps reading, so it should not block the event loop or delay onExit. I have not run it on an older build.
  • My Build Tools install has no Spectre libraries, so I built locally with SpectreMitigation off. binding.gyp is unchanged.

🤖 Generated with Claude Code

The exit watcher erased the pty baton without closing its pseudoconsole,
and kill() does nothing once the baton is gone. Every shell that exited
on its own left a conhost.exe running until the host process exited,
along with any child still attached to that console.

The watcher now closes the pseudoconsole right after it queues the exit
code. kill() nulls hpc once it has closed it, so the watcher never closes
it a second time.

Fixes microsoft#965

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Tyler-V

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

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