From ca0ccf876ff5087b2df1b3c1661cf05248cb8c91 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Sun, 1 Feb 2026 19:41:06 -0800 Subject: [PATCH 01/10] fix: avoid blocking Node.js event loop in ConnectNamedPipe on Windows Fixes #763 The issue was that conptyNative.connect() called ConnectNamedPipe() synchronously for both input and output pipes. While the input pipe was connected via fs.openSync() before the call, the output pipe connection happened asynchronously in a worker thread. If the worker thread hadn't connected yet when ConnectNamedPipe was called, it would block the Node.js event loop waiting for the connection. This caused a deadlock when a debugger was attached (which slows down the event loop and worker thread startup). The fix defers the conptyNative.connect() call until the worker thread signals it has connected to the output pipe (via the onReady callback). This ensures both pipes have clients connected before ConnectNamedPipe is called, so it returns immediately without blocking. --- src/windowsPtyAgent.ts | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 277594d3..fc1b531f 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -42,6 +42,8 @@ export class WindowsPtyAgent { public get innerPid(): number { return this._innerPid; } public get pty(): number { return this._pty; } + private _pendingPtyInfo: { pty: number, commandLine: string, cwd: string, env: string[] } | undefined; + constructor( file: string, args: ArgvOrCommandLine, @@ -78,9 +80,20 @@ export class WindowsPtyAgent { this._outSocket = new Socket(); this._outSocket.setEncoding('utf8'); // The conout socket must be ready out on another thread to avoid deadlocks + // We must wait for the worker to connect before calling conptyNative.connect() + // to avoid blocking the Node.js event loop in ConnectNamedPipe. + // See https://github.com/microsoft/node-pty/issues/763 this._conoutSocketWorker = new ConoutConnection(term.conout, this._useConptyDll); + + // Store pending connection info - we'll complete the connection when worker is ready + this._pendingPtyInfo = { pty: this._pty, commandLine, cwd, env }; + this._conoutSocketWorker.onReady(() => { this._conoutSocketWorker.connectSocket(this._outSocket); + // Now that the worker has connected to the output pipe, we can safely call + // conptyNative.connect() which calls ConnectNamedPipe - it won't block because + // the client (worker) is already connected + this._completePtyConnection(); }); this._outSocket.on('connect', () => { this._outSocket.emit('ready_datapipe'); @@ -93,8 +106,16 @@ export class WindowsPtyAgent { writable: true }); this._inSocket.setEncoding('utf8'); + } + + private _completePtyConnection(): void { + if (!this._pendingPtyInfo) { + return; + } + const { pty, commandLine, cwd, env } = this._pendingPtyInfo; + this._pendingPtyInfo = undefined; - const connect = conptyNative.connect(this._pty, commandLine, cwd, env, this._useConptyDll, c => this._$onProcessExit(c)); + const connect = conptyNative.connect(pty, commandLine, cwd, env, this._useConptyDll, c => this._$onProcessExit(c)); this._innerPid = connect.pid; } From f435bdd58de0049a822f84557a017553b6a4c19f Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Sun, 1 Feb 2026 19:50:07 -0800 Subject: [PATCH 02/10] test: add regression tests for deferred connection fix (#763) --- src/windowsPtyAgent.test.ts | 66 ++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/src/windowsPtyAgent.test.ts b/src/windowsPtyAgent.test.ts index dc2104b3..cf644312 100644 --- a/src/windowsPtyAgent.test.ts +++ b/src/windowsPtyAgent.test.ts @@ -4,7 +4,7 @@ */ import * as assert from 'assert'; -import { argsToCommandLine } from './windowsPtyAgent'; +import { argsToCommandLine, WindowsPtyAgent } from './windowsPtyAgent'; function check(file: string, args: string | string[], expected: string): void { assert.equal(argsToCommandLine(file, args), expected); @@ -91,4 +91,68 @@ if (process.platform === 'win32') { }); }); }); + + describe('WindowsPtyAgent', () => { + describe('connection timing (issue #763)', () => { + it('should defer conptyNative.connect() until worker is ready', function (done) { + this.timeout(10000); + + const term = new WindowsPtyAgent( + 'cmd.exe', + '/c echo test', + Object.keys(process.env).map(k => `${k}=${process.env[k]}`), + process.cwd(), + 80, + 30, + false, + false, + false + ); + + // The innerPid should be 0 initially since connect() is deferred + // until the worker signals ready. This verifies the fix for #763. + const initialPid = term.innerPid; + + // Wait for the connection to complete via ready_datapipe event + term.outSocket.on('ready_datapipe', () => { + // After worker is ready and connect() is called, innerPid should be set + // Use a small delay to ensure _completePtyConnection has run + setTimeout(() => { + assert.notStrictEqual(term.innerPid, 0, 'innerPid should be set after worker is ready'); + assert.strictEqual(initialPid, 0, 'innerPid should have been 0 before worker was ready'); + term.kill(); + done(); + }, 100); + }); + }); + + it('should successfully spawn a process after deferred connection', function (done) { + this.timeout(10000); + + const term = new WindowsPtyAgent( + 'cmd.exe', + '/c echo hello', + Object.keys(process.env).map(k => `${k}=${process.env[k]}`), + process.cwd(), + 80, + 30, + false, + false, + false + ); + + let output = ''; + term.outSocket.on('data', (data: string) => { + output += data; + }); + + // Wait for process to complete and verify output + setTimeout(() => { + assert.ok(output.includes('hello'), `Expected output to contain "hello", got: ${output}`); + term.kill(); + done(); + }, 2000); + }); + }); + }); } From b2e81e9e205389d0e6f2aa1f9817a1ee94216464 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Mon, 2 Feb 2026 19:13:16 -0800 Subject: [PATCH 03/10] test: add non-blocking event loop verification test (#763) --- src/windowsPtyAgent.test.ts | 49 +++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/windowsPtyAgent.test.ts b/src/windowsPtyAgent.test.ts index cf644312..ccc34acd 100644 --- a/src/windowsPtyAgent.test.ts +++ b/src/windowsPtyAgent.test.ts @@ -153,6 +153,55 @@ if (process.platform === 'win32') { done(); }, 2000); }); + + it('should allow async work between construction and connection (non-blocking)', function (done) { + this.timeout(10000); + + // Track the sequence of events to verify non-blocking behavior + const events: string[] = []; + + const term = new WindowsPtyAgent( + 'cmd.exe', + '/c echo test', + Object.keys(process.env).map(k => `${k}=${process.env[k]}`), + process.cwd(), + 80, + 30, + false, + false, + false + ); + + events.push('constructor_returned'); + assert.strictEqual(term.innerPid, 0, 'innerPid should be 0 immediately after construction'); + + // Schedule async work - this MUST run before ready_datapipe if constructor is non-blocking + setImmediate(() => { + events.push('setImmediate_ran'); + // innerPid might still be 0 or might be set by now, depending on timing + // The key is that setImmediate ran, proving the event loop wasn't blocked + }); + + term.outSocket.on('ready_datapipe', () => { + events.push('ready_datapipe'); + + setTimeout(() => { + events.push('final_check'); + + // Verify the sequence: constructor returned, then async work could run + assert.ok(events.includes('constructor_returned'), 'constructor should have returned'); + assert.ok(events.includes('setImmediate_ran'), 'setImmediate should have run (event loop not blocked)'); + assert.ok(events.indexOf('constructor_returned') < events.indexOf('setImmediate_ran'), + 'constructor should return before setImmediate runs'); + + // Most importantly: innerPid should now be set + assert.notStrictEqual(term.innerPid, 0, 'innerPid should be set after connection'); + + term.kill(); + done(); + }, 100); + }); + }); }); }); } From fca43e95f6da4797492b2549f5d0a0058e582079 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Mon, 2 Feb 2026 19:24:59 -0800 Subject: [PATCH 04/10] fix: add timeout fallback for worker connection (#763) If the worker fails to signal ready within 5 seconds, complete the connection anyway to avoid leaving the PTY in a zombie state. --- src/windowsPtyAgent.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index fc1b531f..e5a97788 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -88,7 +88,16 @@ export class WindowsPtyAgent { // Store pending connection info - we'll complete the connection when worker is ready this._pendingPtyInfo = { pty: this._pty, commandLine, cwd, env }; + // Timeout to ensure connection completes even if worker fails to signal ready + const connectionTimeout = setTimeout(() => { + if (this._pendingPtyInfo) { + // Worker never signaled ready - complete connection anyway to avoid zombie state + this._completePtyConnection(); + } + }, 5000); + this._conoutSocketWorker.onReady(() => { + clearTimeout(connectionTimeout); this._conoutSocketWorker.connectSocket(this._outSocket); // Now that the worker has connected to the output pipe, we can safely call // conptyNative.connect() which calls ConnectNamedPipe - it won't block because From aeb7bf8dbc8936e0b5bfadc6d0c4bad62a23dabd Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Mon, 2 Feb 2026 19:29:35 -0800 Subject: [PATCH 05/10] fix: prevent deferred connection after kill() (#763) Clear _pendingPtyInfo in kill() to prevent the timeout or onReady callback from calling connect() on an already-killed PTY handle. --- src/windowsPtyAgent.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index e5a97788..60fcfa30 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -140,6 +140,9 @@ export class WindowsPtyAgent { } public kill(): void { + // Prevent deferred connection from completing after kill + this._pendingPtyInfo = undefined; + // Tell the agent to kill the pty, this releases handles to the process if (!this._useConptyDll) { this._inSocket.readable = false; From f82608a706937a8ba958aaf41a4fc4e46a49fefb Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Mon, 2 Feb 2026 19:38:10 -0800 Subject: [PATCH 06/10] fix: update pid after agent connection completes (#763) The public pid property was only set once at construction, before the deferred connection. Update it in the ready_datapipe handler so users get the correct pid value. --- src/windowsTerminal.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts index 8516750b..0553c333 100644 --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -59,6 +59,8 @@ export class WindowsTerminal extends Terminal { // The forked windows terminal is not available until `ready` event is // emitted. this._socket.on('ready_datapipe', () => { + // Update pid now that the agent has connected + this._pid = this._agent.innerPid; // Run deferreds and set ready state once the first data event is received. this._socket.once('data', () => { From 853fea373488df8a6d161442a2f0b85c869e3df3 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Mon, 2 Feb 2026 20:42:51 -0800 Subject: [PATCH 07/10] test: verify pid is set after ready_datapipe (#763) Add test to ensure WindowsTerminal.pid is correctly updated after the deferred connection completes. This closes the test coverage gap for the public pid property. --- src/windowsTerminal.test.ts | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index 29f0411d..a368480e 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -126,6 +126,32 @@ if (process.platform === 'win32') { }); }); + describe('pid', () => { + it('should be 0 before ready and set after ready_datapipe (issue #763)', function (done) { + this.timeout(10000); + const term = new WindowsTerminal('cmd.exe', '/c echo test', { useConptyDll }); + + // pid may be 0 immediately after construction due to deferred connection + const initialPid = term.pid; + + // Access internal socket to listen for ready_datapipe + const socket = (term as any)._socket; + socket.on('ready_datapipe', () => { + // After ready_datapipe, pid should be set to a valid non-zero value + setTimeout(() => { + assert.notStrictEqual(term.pid, 0, 'pid should be set after ready_datapipe'); + assert.strictEqual(typeof term.pid, 'number', 'pid should be a number'); + // If initial was 0, it should now be different (proves the fix works) + if (initialPid === 0) { + assert.notStrictEqual(term.pid, initialPid, 'pid should be updated from initial value'); + } + term.on('exit', () => done()); + term.kill(); + }, 100); + }); + }); + }); + describe('resize', () => { it('should throw a non-native exception when resizing an invalid value', function(done) { this.timeout(20000); From 2b25c76157fdfe02be4387e892e850ae26b33c2f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 3 Feb 2026 04:12:06 -0800 Subject: [PATCH 08/10] Don't get process list on kill if not yet connected --- src/conpty_console_list_agent.ts | 10 +++++++++- src/windowsPtyAgent.ts | 3 +++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/conpty_console_list_agent.ts b/src/conpty_console_list_agent.ts index 181ccabb..5cac1ffa 100644 --- a/src/conpty_console_list_agent.ts +++ b/src/conpty_console_list_agent.ts @@ -10,6 +10,14 @@ import { loadNativeModule } from './utils'; const getConsoleProcessList = loadNativeModule('conpty_console_list').module.getConsoleProcessList; const shellPid = parseInt(process.argv[2], 10); -const consoleProcessList = getConsoleProcessList(shellPid); +let consoleProcessList: number[] = []; +if (shellPid > 0) { + try { + consoleProcessList = getConsoleProcessList(shellPid); + } catch { + // AttachConsole can fail if the process already exited or is invalid. + consoleProcessList = []; + } +} process.send!({ consoleProcessList }); process.exit(0); diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 60fcfa30..cdc80415 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -169,6 +169,9 @@ export class WindowsPtyAgent { } private _getConsoleProcessList(): Promise { + if (this._innerPid <= 0) { + return Promise.resolve([]); + } return new Promise(resolve => { const agent = fork(path.join(__dirname, 'conpty_console_list_agent'), [ this._innerPid.toString() ]); agent.on('message', message => { From f88917a314e842982a4cf7c2b7c41ccdf0f018ef Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 3 Feb 2026 04:14:45 -0800 Subject: [PATCH 09/10] Make lint happy --- src/conpty_console_list_agent.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/conpty_console_list_agent.ts b/src/conpty_console_list_agent.ts index 5cac1ffa..68fc5d4b 100644 --- a/src/conpty_console_list_agent.ts +++ b/src/conpty_console_list_agent.ts @@ -12,12 +12,12 @@ const getConsoleProcessList = loadNativeModule('conpty_console_list').module.get const shellPid = parseInt(process.argv[2], 10); let consoleProcessList: number[] = []; if (shellPid > 0) { - try { - consoleProcessList = getConsoleProcessList(shellPid); - } catch { - // AttachConsole can fail if the process already exited or is invalid. - consoleProcessList = []; - } + try { + consoleProcessList = getConsoleProcessList(shellPid); + } catch { + // AttachConsole can fail if the process already exited or is invalid. + consoleProcessList = []; + } } process.send!({ consoleProcessList }); process.exit(0); From 4a1433ab6606c066ad951d9582aaf324a1f9041b Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 3 Feb 2026 04:32:48 -0800 Subject: [PATCH 10/10] Prevent OOM in test --- src/windowsTerminal.test.ts | 56 ++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index a368480e..28c102bc 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -104,25 +104,43 @@ if (process.platform === 'win32') { it('should kill the process tree', function (done: Mocha.Done): void { this.timeout(20000); const term = new WindowsTerminal('cmd.exe', [], { useConptyDll }); - // Start sub-processes - term.write('powershell.exe\r'); - term.write('node.exe\r'); - console.log('start poll for tree size'); - pollForProcessTreeSize(term.pid, 3, 500, 5000).then(list => { - assert.strictEqual(list[0].name.toLowerCase(), 'cmd.exe'); - assert.strictEqual(list[1].name.toLowerCase(), 'powershell.exe'); - assert.strictEqual(list[2].name.toLowerCase(), 'node.exe'); - term.kill(); - const desiredState: IProcessState = {}; - desiredState[list[0].pid] = false; - desiredState[list[1].pid] = false; - desiredState[list[2].pid] = false; - term.on('exit', () => { - pollForProcessState(desiredState, 1000, 5000).then(() => { - done(); - }).catch(done); - }); - }).catch(done); + const socket = (term as any)._socket; + let started = false; + const startPolling = (): void => { + if (started) { + return; + } + if (term.pid === 0) { + setTimeout(startPolling, 50); + return; + } + started = true; + // Start sub-processes + term.write('powershell.exe\r'); + term.write('node.exe\r'); + console.log('start poll for tree size'); + pollForProcessTreeSize(term.pid, 3, 500, 5000).then(list => { + assert.strictEqual(list[0].name.toLowerCase(), 'cmd.exe'); + assert.strictEqual(list[1].name.toLowerCase(), 'powershell.exe'); + assert.strictEqual(list[2].name.toLowerCase(), 'node.exe'); + term.kill(); + const desiredState: IProcessState = {}; + desiredState[list[0].pid] = false; + desiredState[list[1].pid] = false; + desiredState[list[2].pid] = false; + term.on('exit', () => { + pollForProcessState(desiredState, 1000, 5000).then(() => { + done(); + }).catch(done); + }); + }).catch(done); + }; + + if (term.pid > 0) { + startPolling(); + } else { + socket.once('ready_datapipe', () => setTimeout(startPolling, 50)); + } }); });