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:
- Spawn a pty running a short command that exits on its own (
cmd.exe /c echo hi).
- Let it exit naturally; do not call
ptyProcess.kill().
- Repeat N times.
- 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
Summary
On Windows/ConPTY, when a shell exits on its own (rather than via
kill()),ClosePseudoConsoleis never called. Thepty_batonis erased by the exit-watcher thread before the JSonExitis delivered,struct pty_batonhas no destructor, andPtyKill— the only JS-reachable route toClosePseudoConsole— 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 callkill()and still find a live baton.Present on
maintoday, and on 1.2.0-beta.10 / 1.2.0-beta.14.Where
src/win/conpty.cconmain:init()exports onlystartProcess | connect | resize | clear | kill, andpfnClosePseudoConsoleis called from exactly one place, soPtyKillreally is the only way for a consumer to reach it.Worth noting the defensive
CloseHandle(handle->hIn / handle->hOut)recently added insidePtyKillis 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:
Under
NDEBUG,assert(expr)expands to((void)0)andremove_pty_batonnever runs — the baton survives andPtyKillcloses the HPCON correctly. So on that version the leak exists only when asserts are enabled.They are enabled in the published binaries.
strings -a -elon@lydell/node-pty-win32-x64@1.2.0-beta.10'sprebuilds/win32-x64/conpty.node(which is built fromnode-pty@1.2.0-beta.10) yields, in UTF-16LE:The literal expression text is only emitted for a compiled-in MSVC assert, and
binding.gypdefines noNDEBUG.Hoisting it out of the
assertto an unconditionalstd::erase_if(done at 1.2.0-beta.14,:101, still beforeBlockingCallat: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-pty1.2.0-beta.10):conhost.exe --headlessunder a single host process after ~12 h, ~2.8 GB working set.The same report shows the parent's thread count growing in lockstep (353 threads against 347 orphans) — that is the per-pty
ConoutConnectionworker, whichConoutConnection.dispose()frees and which is likewise reachable only fromkill(). Downstream can fix that half by callingdispose()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:
cmd.exe /c echo hi).ptyProcess.kill().conhost.exe --headlesschildren 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:pty_batona destructor that callsClosePseudoConsole(hpc)(and closeshIn/hOutif still open), sostd::erase_if/unique_ptrteardown does the right thing everywhere; orClosePseudoConsole(baton->hpc)explicitly in the watcher, immediately before theerase_if.The destructor is the more robust of the two — it also covers the
remove_pty_batoncall sites and any future erase — but it needsLoadConptyDll'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
kill()path; independent of this one, which is about the path wherekill()is never called at all.