Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .ai/contexts/ipc-bridge.md
Original file line number Diff line number Diff line change
Expand Up @@ -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] <op> skipped session=<id> reason=<message>`. Debug level
is deliberate — the file transport is at `info` in packaged builds, so a resize
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions pty-ops.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
63 changes: 63 additions & 0 deletions test/pty-ops-conpty-kill.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// 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 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(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(() => {});
setTimeout(() => {
const session = { pty: term };
killPty(session, 's1');
killPty(session, 's1');
setTimeout(() => process.exit(0), 1500);
}, 1000);
`;

// The packaged app finds conpty.dll beside the conpty.node it loads (scripts/after-pack.js
// copies prebuilds/win32-<arch>/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) {
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',
{ 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}`);
});
23 changes: 21 additions & 2 deletions test/pty-ops.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ─────────────────────────────────────────────
Expand Down Expand Up @@ -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']]);
});
Loading