ConPTY kill(): the console-list agent races the pseudoconsole teardown, so the leftover-process sweep never runs

[hili986]

Summary

On the ConPTY path, WindowsPtyAgent.kill() forks the console-list agent and then destroys the pseudoconsole on the next statement. The agent frequently loses that race, throws AttachConsole failed at module scope onto the inherited stderr, and never sends its message — so the 5000 ms timeout resolves [this._innerPid] and the leftover-process sweep runs five seconds late against a pid that was already killed.

The sweep exists to prevent detached console processes surviving a kill (the comment on the winpty branch cites microsoft/vscode#26807). Measured here, it does not prevent them.

The code

lib/windowsPtyAgent.js, kill():

this._getConsoleProcessList().then(consoleProcessList => {
  consoleProcessList.forEach(pid => { try { process.kill(pid) } catch (e) {} })
})
this._ptyNative.kill(this._pty, this._useConptyDll)   // synchronous, next statement
this._conoutSocketWorker.dispose()

_getConsoleProcessList() forks conpty_console_list_agent, which calls getConsoleProcessList(shellPid) at module scope. src/win/conpty_console_list.cc throws when AttachConsole fails:

if (!AttachConsole(pid)) {
  throw Napi::Error::New(env, "AttachConsole failed");
}

Forking a node process takes long enough that the pseudoconsole is often gone before the agent attaches. Two consequences follow from that throw:

1. The agent dies without process.send, so the promise falls through to the 5000 ms timeout and resolves [this._innerPid] — a pid _ptyNative.kill has already dealt with. The sweep becomes a no-op.
2. fork() defaults to silent: false, so the child's stderr is inherited. The uncaught throw prints a full stack trace into the host application's stderr, from a path the host did not call and cannot catch.

Measurements

node-pty 1.1.0, Node 24.13.1, Windows 11 26200, useConptyDll false (the default).

Frequency. 12 rounds of pty.spawn + kill(), nothing else in the process — 5 of 12 printed the stack trace:

node_modules\node-pty\lib\conpty_console_list_agent.js:13
var consoleProcessList = getConsoleProcessList(shellPid);
                         ^
Error: AttachConsole failed
    at Object.<anonymous> (...\lib\conpty_console_list_agent.js:13:26)

Leak. Spawn a shell, run a pipeline, kill(), wait 6 s — past the 5 s fallback. A stage survived in 5 of 5 rounds:

round 0: 3 pipeline stages before kill -> 1 still alive 6s after kill
round 1: 3 -> 1
round 2: 3 -> 1
round 3: 3 -> 1
round 4: 3 -> 1

Repro:

const pty = require('node-pty')
const delay = ms => new Promise(r => setTimeout(r, ms))

for (let i = 0; i < 5; i++) {
  const p = pty.spawn('C:\\Program Files\\Git\\usr\\bin\\bash.exe', ['--noprofile', '--norc', '-i'], { cols: 100, rows: 24 })
  p.onData(() => {})
  await delay(700)
  p.write('sleep 60 | cat\n')
  await delay(1200)
  // count sleep.exe / cat.exe here
  p.kill()
  await delay(6000)
  // count again — survivors remain
}

Why it is worth fixing rather than silencing

A consumer cannot work around either half. The stack trace comes from a forked child it did not create, so it cannot be caught or redirected. The failed sweep is silent — the promise resolves normally with a shorter list, so there is no signal that the console list was never read.

We compensate downstream by resolving the console list ourselves at signal time and fanning out over it, but that only covers our own signal path; node-pty's kill still leaves what it leaves.

Suggested fixes

Smallest correct change is to read the console list before destroying the pseudoconsole, since that is the only moment the console is guaranteed to exist:

const consoleProcessList = await this._getConsoleProcessList()
this._ptyNative.kill(this._pty, this._useConptyDll)
consoleProcessList.forEach(pid => { try { process.kill(pid) } catch (e) {} })

This makes kill() async, which may not be acceptable. Two smaller changes are independently worth having either way:

• Catch in the agent. conpty_console_list_agent.js should wrap the call and process.send({ consoleProcessList: [] }) on failure, so a lost race costs nothing instead of five seconds and a stack trace.
• Pass silent: true to fork(). A helper's failure should not print into the host's stderr from a path the host never called.

Context

Found while investigating a stray stack trace in CI for a Windows plugin that supplies a ProcessInspector on top of node-pty's ConPTY. Full analysis, including the control that ruled out our own console probe as the source: sjh9714/dsh-win32#26.