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

Iniziate in src/win/conpty.cc leggendo pty_baton, SetupExitCallback e PtyKill per tracciare la distruzione del baton e la pulizia della pseudoconsole. Eseguite la riproduzione su Windows con processi cmd.exe che terminano naturalmente, quindi verificate che le istanze di conhost.exe non si accumulino, mentre il percorso «kill-before-exit» continui a funzionare correttamente.

Summary

On Windows/ConPTY, when a shell exits on its own (rather than via kill()), ClosePseudoConsole is never called. The pty_baton is erased by the exit-watcher thread before the JS onExit is delivered, struct pty_baton has no destructor, and PtyKill — the only JS-reachable route to ClosePseudoConsole — silently no-ops once the baton is gone.

The result is one orphaned conhost.exe --headless (~8 MB working set) per pty, held for the lifetime of the host process. Because the erase happens before any JS callback runs, no client of node-pty can work around this: there is no point at which a consumer could call kill() and still find a live baton.

Present on main today, and on 1.2.0-beta.10 / 1.2.0-beta.14.

Where

src/win/conpty.cc on main:

// struct has a ctor, no dtor — nothing closes hpc when the unique_ptr dies
:43-52   struct pty_baton {
           int id; HANDLE hIn; HANDLE hOut; HPCON hpc; HANDLE hShell = nullptr;
           pty_baton(int _id, HANDLE _hIn, HANDLE _hOut, HPCON _hpc) : ... {};
         };
:54      static std::vector<std::unique_ptr<pty_baton>> ptyHandles;

// SetupExitCallback's watcher thread
:95      WaitForSingleObject(baton->hShell, INFINITE);
:96-104  {
           std::lock_guard<std::mutex> lock(g_ptyHandlesMutex);
           GetExitCodeProcess(...); CloseHandle(baton->hShell);
           std::erase_if(ptyHandles, ...);      // <-- baton destroyed here; hpc NOT closed
         }
:106     auto status = tsfn.BlockingCall(exit_event, callback);  // <-- JS onExit, AFTER the erase

// PtyKill — the only exported route to ClosePseudoConsole
:569-571 std::lock_guard<std::mutex> lock(g_ptyHandlesMutex);
         pty_baton* handle = get_pty_baton(lock, id);
:572     if (handle != nullptr) {
:582       pfnClosePseudoConsole(handle->hpc);  // <-- skipped when the baton is gone
:599     return env.Undefined();                // <-- silent: no throw, no diagnostic

init() exports only startProcess | connect | resize | clear | kill, and pfnClosePseudoConsole is called from exactly one place, so PtyKill really is the only way for a consumer to reach it.

Worth noting the defensive CloseHandle(handle->hIn / handle->hOut) recently added inside PtyKill is unreachable on this same path, for the same reason.

On 1.2.0-beta.10, the erase is a side effect inside assert()

The shipped beta line has this shape, which is worth calling out because it inverts the usual expectation:

:105   CloseHandle(baton->hShell);
:106   assert(remove_pty_baton(baton->id));
:108   auto status = tsfn.BlockingCall(exit_event, callback);

Under NDEBUG, assert(expr) expands to ((void)0) and remove_pty_baton never runs — the baton survives and PtyKill closes the HPCON correctly. So on that version the leak exists only when asserts are enabled.

They are enabled in the published binaries. strings -a -el on @lydell/node-pty-win32-x64@1.2.0-beta.10's prebuilds/win32-x64/conpty.node (which is built from node-pty@1.2.0-beta.10) yields, in UTF-16LE:

remove_pty_baton(baton->id)
Assertion failed: %Ts, file %Ts, line %d
D:\a\_work\1\s\src\win\conpty.cc

The literal expression text is only emitted for a compiled-in MSVC assert, and binding.gyp defines no NDEBUG.

Hoisting it out of the assert to an unconditional std::erase_if (done at 1.2.0-beta.14, :101, still before BlockingCall at :106) fixes the side-effect-in-assert anti-pattern, but it makes the erase reliable and therefore makes the HPCON leak deterministic on every build rather than only on assert-enabled ones.

Field evidence

From a downstream report against a CLI that creates one pty per shell command (QwenLM/qwen-code#11303 — that project pins @lydell/node-pty 1.2.0-beta.10):

• 347 orphaned conhost.exe --headless under a single host process after ~12 h, ~2.8 GB working set.
• Growth is 1:1 with pty creations: +7 orphans for exactly 7 shell commands, across two independent samples.
• The processes die only when the host process itself exits.

The same report shows the parent's thread count growing in lockstep (353 threads against 347 orphans) — that is the per-pty ConoutConnection worker, which ConoutConnection.dispose() frees and which is likewise reachable only from kill(). Downstream can fix that half by calling dispose() directly; the HPCON half has no such workaround.

Reproduction

I do not have a Windows machine, so I have not run this myself — the analysis above is from the source at the affected versions plus the shipped prebuild's strings, and the counts are the downstream reporter's. A maintainer should be able to confirm quickly:

1. Spawn a pty running a short command that exits on its own (cmd.exe /c echo hi).
2. Let it exit naturally; do not call ptyProcess.kill().
3. Repeat N times.
4. Count conhost.exe --headless children of the host process — it should grow by N and never shrink.

Contrast with the same loop where kill() is called before the shell exits, which closes the HPCON correctly.

Suggested fix

Close the pseudoconsole where the baton is destroyed, so it does not depend on a consumer calling kill() in a window that no longer exists. Either:

• give pty_baton a destructor that calls ClosePseudoConsole(hpc) (and closes hIn/hOut if still open), so std::erase_if / unique_ptr teardown does the right thing everywhere; or
• call ClosePseudoConsole(baton->hpc) explicitly in the watcher, immediately before the erase_if.

The destructor is the more robust of the two — it also covers the remove_pty_baton call sites and any future erase — but it needs LoadConptyDll's function pointer to be resolvable from that context, which is why the explicit call in the watcher may be the smaller change.

Either way PtyKill's existing close stays correct for the kill-before-exit path, and becomes a no-op-after-close rather than the only close.

Related

• #333 — the same family, for the shell process rather than the pseudoconsole host.
• #947, #952 — other leaks/races on the ConPTY kill() path; independent of this one, which is about the path where kill() is never called at all.