From 124deae82b674165958874a382fa08ebdd2e3e86 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Thu, 1 Oct 2026 22:38:50 +0200 Subject: [PATCH 1/3] (terminal): close a pty once, and stop resizing or writing to a killed one A second kill issued before the shell's exit event reaches node-pty's native kill again, which calls ConptyClosePseudoConsole on an already freed pseudo console handle and corrupts the heap (0xc0000374), taking the whole main process down with no JS error. killPty now closes a given pty once; resizePty and writePty skip a pty that was killed. Closes #405 --- CHANGELOG.md | 3 +++ pty-ops.js | 11 +++++++++++ test/pty-ops.test.js | 23 +++++++++++++++++++++-- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97dae2a5..4dcfbf2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ## Unreleased +### Fixed +- Stopping a terminal twice in quick succession, or resizing it while it is being stopped, no longer closes the Windows pseudo console twice, which could kill the whole app with no error. (#405) + ## v0.0.86 — 2026-10-01 ### New diff --git a/pty-ops.js b/pty-ops.js index d92fcb1a..530bee30 100644 --- a/pty-ops.js +++ b/pty-ops.js @@ -5,6 +5,9 @@ const os = require('os'); let logger = null; +// see .ai/contexts/ipc-bridge.md ("PTY operations race the exit") +const killedPtys = new WeakSet(); + /** Install the sink used to report swallowed PTY errors. `null` disables it. */ function setPtyOpLogger(next) { logger = next && typeof next.debug === 'function' ? next : null; @@ -34,15 +37,23 @@ function withPty(session, label, fn, sessionId) { } } +function isKilled(session) { + return !!(session && session.pty && killedPtys.has(session.pty)); +} + function resizePty(session, cols, rows, sessionId) { + if (isKilled(session)) return false; return withPty(session, 'resize', (pty) => pty.resize(cols, rows), sessionId); } function killPty(session, sessionId) { + if (!session || !session.pty || killedPtys.has(session.pty)) return false; + killedPtys.add(session.pty); return withPty(session, 'kill', (pty) => pty.kill(), sessionId); } function writePty(session, data, sessionId) { + if (isKilled(session)) return false; return withPty(session, 'write', (pty) => pty.write(data), sessionId); } diff --git a/test/pty-ops.test.js b/test/pty-ops.test.js index 81b88957..fd9fc5bc 100644 --- a/test/pty-ops.test.js +++ b/test/pty-ops.test.js @@ -43,9 +43,9 @@ test('resizePty forwards cols/rows to a live pty and reports success', () => { test('killPty and writePty forward to a live pty and report success', () => { const pty = makeLivePty(); - assert.equal(killPty({ pty }, 's1'), true); assert.equal(writePty({ pty }, 'hi', 's1'), true); - assert.deepEqual(pty.calls, [['kill'], ['write', 'hi']]); + assert.equal(killPty({ pty }, 's1'), true); + assert.deepEqual(pty.calls, [['write', 'hi'], ['kill']]); }); // ── The race the crash came from ───────────────────────────────────────────── @@ -140,3 +140,22 @@ test('a keystroke to a pty that exits mid-write does not escape handleTerminalIn // The composer still saw the keystroke — the guard must not skip the bookkeeping. assert.equal(session.composerState.pending, 5); }); + +// ── A killed pty is closed once (double ClosePseudoConsole corrupts the heap) ── + +test('killPty kills a live pty once and ignores later kills', () => { + const pty = makeLivePty(); + const session = { pty }; + assert.equal(killPty(session, 's1'), true); + assert.equal(killPty(session, 's1'), false); + assert.deepEqual(pty.calls, [['kill']]); +}); + +test('resizePty and writePty do nothing once the pty was killed', () => { + const pty = makeLivePty(); + const session = { pty }; + killPty(session, 's1'); + assert.equal(resizePty(session, 80, 24, 's1'), false); + assert.equal(writePty(session, 'x', 's1'), false); + assert.deepEqual(pty.calls, [['kill']]); +}); From c149c71b7b8cca5fd8a020452d9ef1be3f6450d8 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Thu, 1 Oct 2026 23:14:56 +0200 Subject: [PATCH 2/3] (terminal): prove the double kill against the real ConPTY, and document it A child node process loads the real node-pty with useConptyDll and kills a pty twice back to back; with the guard removed it dies with 0xc0000374. The ipc-bridge context now records the once-only kill. Refs #405 --- .ai/contexts/ipc-bridge.md | 10 ++++++++ test/pty-ops-conpty-kill.test.js | 41 ++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 test/pty-ops-conpty-kill.test.js diff --git a/.ai/contexts/ipc-bridge.md b/.ai/contexts/ipc-bridge.md index e6988ff2..e60d4268 100644 --- a/.ai/contexts/ipc-bridge.md +++ b/.ai/contexts/ipc-bridge.md @@ -215,6 +215,16 @@ write is bounded by the try/catch on the write"). Callers get a boolean instead of an exception; the first-resize nudge uses it to skip its follow-up when the PTY is already gone. +A kill is once-only. With `useConptyDll`, node-pty's `kill` ends in +`ConptyClosePseudoConsole(hpc)`, and the native handle is only dropped when the +shell's exit thread runs; a second kill before that finds the handle still +registered and closes an already-freed pseudo console, a heap corruption +(0xc0000374) that kills the whole main process with no JS error. So `killPty` +closes a given pty once (a `WeakSet` of killed ptys), and `resizePty` / +`writePty` return false on a killed pty. `test/pty-ops-conpty-kill.test.js` +proves it against the real node-pty in a child process (Windows only): with the +guard removed the child dies with 3221226356. + Swallowed errors are not silent: `setPtyOpLogger(log)` in `main.js` routes them to `log.debug` as `[pty] skipped session= reason=`. Debug level is deliberate — the file transport is at `info` in packaged builds, so a resize diff --git a/test/pty-ops-conpty-kill.test.js b/test/pty-ops-conpty-kill.test.js new file mode 100644 index 00000000..dae6820c --- /dev/null +++ b/test/pty-ops-conpty-kill.test.js @@ -0,0 +1,41 @@ +// test/pty-ops-conpty-kill.test.js — the real node-pty + bundled conpty.dll, in a child process. +// A second ClosePseudoConsole on the same handle corrupts the heap (0xc0000374) and takes the +// process down; the child is the only place that can be observed. see .ai/contexts/ipc-bridge.md +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); + +const HEAP_CORRUPTION = 3221226356; // 0xc0000374 + +const CHILD = ` +const pty = require('node-pty'); +const { killPty } = require(process.env.PTY_OPS); +const term = pty.spawn(process.env.ComSpec || 'cmd.exe', [], { cols: 80, rows: 24, useConptyDll: true }); +term.onData(() => {}); +setTimeout(() => { + const session = { pty: term }; + killPty(session, 's1'); + killPty(session, 's1'); + setTimeout(() => process.exit(0), 1500); +}, 1000); +`; + +function runChild(extraEnv) { + return spawnSync(process.execPath, ['-e', CHILD], { + cwd: path.join(__dirname, '..'), + env: { ...process.env, PTY_OPS: path.join(__dirname, '..', 'pty-ops.js'), ...extraEnv }, + timeout: 30000, + encoding: 'utf8', + }); +} + +test('killing a conpty-dll pty twice back to back does not corrupt the heap', + { skip: process.platform !== 'win32' ? 'ConptyClosePseudoConsole exists only on Windows' : false }, + () => { + const r = runChild({}); + assert.notEqual(r.status, HEAP_CORRUPTION, 'child died of STATUS_HEAP_CORRUPTION'); + assert.equal(r.status, 0, `child stderr: ${r.stderr}`); + }); From 5332ec1a66f65d14214bef255683d50edfc95cb0 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Thu, 1 Oct 2026 23:27:31 +0200 Subject: [PATCH 3/3] (terminal): stage a packaged-style node-pty layout for the double-kill test On CI node-pty is built into build/Release, which has no conpty/ folder (the afterPack hook adds it at packaging time), so the child could not find conpty.dll. The child now loads a temp copy of node-pty that carries only the prebuilds, the layout the packaged app resolves. Refs #405 --- test/pty-ops-conpty-kill.test.js | 36 +++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/test/pty-ops-conpty-kill.test.js b/test/pty-ops-conpty-kill.test.js index dae6820c..d91ce0eb 100644 --- a/test/pty-ops-conpty-kill.test.js +++ b/test/pty-ops-conpty-kill.test.js @@ -6,12 +6,14 @@ const test = require('node:test'); const assert = require('node:assert/strict'); const path = require('node:path'); +const fs = require('node:fs'); +const os = require('node:os'); const { spawnSync } = require('node:child_process'); const HEAP_CORRUPTION = 3221226356; // 0xc0000374 const CHILD = ` -const pty = require('node-pty'); +const pty = require(process.env.NODE_PTY_DIR); const { killPty } = require(process.env.PTY_OPS); const term = pty.spawn(process.env.ComSpec || 'cmd.exe', [], { cols: 80, rows: 24, useConptyDll: true }); term.onData(() => {}); @@ -23,13 +25,33 @@ setTimeout(() => { }, 1000); `; +// The packaged app finds conpty.dll beside the conpty.node it loads (scripts/after-pack.js +// copies prebuilds/win32-/conpty/ there). A source-built node_modules has no such +// folder in build/Release, so the child loads a temp copy that has only the prebuilds. +function stageNodePty() { + const src = path.dirname(require.resolve('node-pty/package.json')); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'sb-nodepty-')); + const dst = path.join(dir, 'node-pty'); + const prebuild = path.join('prebuilds', `${process.platform}-${process.arch}`); + fs.cpSync(path.join(src, 'lib'), path.join(dst, 'lib'), { recursive: true }); + fs.cpSync(path.join(src, prebuild), path.join(dst, prebuild), { recursive: true }); + fs.copyFileSync(path.join(src, 'package.json'), path.join(dst, 'package.json')); + return { dir, nodePty: dst }; +} + function runChild(extraEnv) { - return spawnSync(process.execPath, ['-e', CHILD], { - cwd: path.join(__dirname, '..'), - env: { ...process.env, PTY_OPS: path.join(__dirname, '..', 'pty-ops.js'), ...extraEnv }, - timeout: 30000, - encoding: 'utf8', - }); + const staged = stageNodePty(); + try { + return spawnSync(process.execPath, ['-e', CHILD], { + cwd: path.join(__dirname, '..'), + env: { ...process.env, PTY_OPS: path.join(__dirname, '..', 'pty-ops.js'), NODE_PTY_DIR: staged.nodePty, ...extraEnv }, + timeout: 30000, + encoding: 'utf8', + }); + } finally { + // OpenConsole.exe may still hold the copy open for a moment; the temp dir is disposable. + try { fs.rmSync(staged.dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 300 }); } catch {} + } } test('killing a conpty-dll pty twice back to back does not corrupt the heap',