diff --git a/binding.gyp b/binding.gyp index 5f63978b07ab50aaf7523219a2170ec737a6b5db..e23988358eecc292ca769a738d5bfdef8172bb3d 100644 --- a/binding.gyp +++ b/binding.gyp @@ -1,4 +1,7 @@ { + 'variables': { + 'node_pty_test_faults%': 0, + }, 'target_defaults': { 'dependencies': [ " nul"', environment_1, process.cwd(), 80, 30, false, true, true, false, true); }; + var runFailureCase_1 = function (failure, expectedError) { + assert.ok(native_1.setTestTerminateJobFailure && native_1.getTestResourceState); + var agent = createAgent_1(); + var before = native_1.getTestResourceState(); + native_1.setTestTerminateJobFailure(failure); + try { + assert.throws(function () { return agent.terminateJobTree(1000); }, expectedError); + var after = native_1.getTestResourceState(); + assert.strictEqual(after.closedTerminationJobHandleCount, before.closedTerminationJobHandleCount + 1); + assert.strictEqual(after.liveTerminationThreadCount, before.liveTerminationThreadCount); + } + finally { + native_1.setTestTerminateJobFailure('none'); + agent.kill(); + } + }; + var runAssertions_1 = function () { + assert.ok(native_1.setTestTerminateJobFailure && native_1.getTestResourceState, 'ConPTY termination test fault injection was required but is unavailable'); + assert.throws(function () { return new windowsPtyAgent_1.WindowsPtyAgent(process.env.ComSpec || 'cmd.exe', '/C exit', environment_1, process.cwd(), 80, 30, false, false, false, false, true); }, /Windows agent Job ownership requires ConPTY/); + runFailureCase_1('thread-allocation', /Injected termination thread allocation failure/); + runFailureCase_1('threadsafe-function', /Injected termination ThreadSafeFunction failure/); + }; + if (typeof describe === 'function') { + describe('ConPTY Job capability and termination cleanup', function () { + it('fails closed without ConPTY and releases allocation-failure resources', function () { + runAssertions_1(); + }); + }); + } + else { + var leakedWorkerWatchdog = setTimeout(function () { + throw new Error('ConPTY Job termination cleanup left JS resources alive'); + }, 5000); + leakedWorkerWatchdog.unref(); + runAssertions_1(); + console.log('ConPTY Job capability and termination cleanup: 3 passed'); + } +} diff --git a/lib/conpty_console_list_agent.js b/lib/conpty_console_list_agent.js index 8c4fca9022a6d6f015bca87f61625cde2278f428..201fc1d70a907e31b5a2b33b19a9db20803bc652 100644 --- a/lib/conpty_console_list_agent.js +++ b/lib/conpty_console_list_agent.js @@ -10,7 +10,14 @@ Object.defineProperty(exports, "__esModule", { value: true }); var utils_1 = require("./utils"); var getConsoleProcessList = utils_1.loadNativeModule('conpty_console_list').module.getConsoleProcessList; var shellPid = parseInt(process.argv[2], 10); -var consoleProcessList = getConsoleProcessList(shellPid); +var consoleProcessList; +try { + consoleProcessList = getConsoleProcessList(shellPid); +} +catch (_a) { + // Why: AttachConsole can fail after the shell exits; parent already has this fallback. + consoleProcessList = [shellPid]; +} process.send({ consoleProcessList: consoleProcessList }); process.exit(0); //# sourceMappingURL=conpty_console_list_agent.js.map \ No newline at end of file diff --git a/lib/unixTerminal.js b/lib/unixTerminal.js index 1ec12f796a822c78fba9ad7f6448c3987e325c23..cec8b67aef02f8199e5606a0d257088bf1865877 100644 --- a/lib/unixTerminal.js +++ b/lib/unixTerminal.js @@ -28,8 +28,12 @@ var native = utils_1.loadNativeModule('pty'); var pty = native.module; var helperPath = native.dir + '/spawn-helper'; helperPath = path.resolve(__dirname, helperPath); -helperPath = helperPath.replace('app.asar', 'app.asar.unpacked'); -helperPath = helperPath.replace('node_modules.asar', 'node_modules.asar.unpacked'); +if (!helperPath.includes('app.asar.unpacked')) { + helperPath = helperPath.replace('app.asar', 'app.asar.unpacked'); +} +if (!helperPath.includes('node_modules.asar.unpacked')) { + helperPath = helperPath.replace('node_modules.asar', 'node_modules.asar.unpacked'); +} var DEFAULT_FILE = 'sh'; var DEFAULT_NAME = 'xterm'; var DESTROY_SOCKET_TIMEOUT_MS = 200; diff --git a/lib/windowsPtyAgent.js b/lib/windowsPtyAgent.js index a358ffb177357e177661033c1b092f9c9d0e5f5a..7ce4a29defba89c972cb405035f396041eb96695 100644 --- a/lib/windowsPtyAgent.js +++ b/lib/windowsPtyAgent.js @@ -26,17 +26,26 @@ var FLUSH_DATA_INTERVAL = 1000; * and winpty. */ var WindowsPtyAgent = /** @class */ (function () { - function WindowsPtyAgent(file, args, env, cwd, cols, rows, debug, _useConpty, _useConptyDll, conptyInheritCursor) { + function WindowsPtyAgent(file, args, env, cwd, cols, rows, debug, _useConpty, _useConptyDll, conptyInheritCursor, windowsAgentJob) { var _this = this; if (_useConptyDll === void 0) { _useConptyDll = false; } if (conptyInheritCursor === void 0) { conptyInheritCursor = false; } + if (windowsAgentJob === void 0) { windowsAgentJob = false; } this._useConpty = _useConpty; this._useConptyDll = _useConptyDll; this._pid = 0; this._innerPid = 0; + this._processExited = false; + this._windowsAgentJob = false; + this._processExitListeners = []; if (this._useConpty === undefined || this._useConpty === true) { this._useConpty = this._getWindowsBuildNumber() >= 18309; } + // Why: winpty cannot provide spawn-time Job ownership; launching anyway can orphan agents. + if (windowsAgentJob && !this._useConpty) { + throw new Error('Windows agent Job ownership requires ConPTY'); + } + this._windowsAgentJob = windowsAgentJob; if (this._useConpty) { if (!conptyNative) { conptyNative = utils_1.loadNativeModule('conpty').module; @@ -72,7 +81,7 @@ var WindowsPtyAgent = /** @class */ (function () { this._outSocket.setEncoding('utf8'); // The conout socket must be ready out on another thread to avoid deadlocks this._conoutSocketWorker = new windowsConoutConnection_1.ConoutConnection(term.conout, this._useConptyDll); - this._conoutSocketWorker.onReady(function () { + var conoutReady = this._conoutSocketWorker.onReady(function () { _this._conoutSocketWorker.connectSocket(_this._outSocket); }); this._outSocket.on('connect', function () { @@ -86,8 +95,19 @@ var WindowsPtyAgent = /** @class */ (function () { }); this._inSocket.setEncoding('utf8'); if (this._useConpty) { - var connect = this._ptyNative.connect(this._pty, commandLine, cwd, env, this._useConptyDll, function (c) { return _this._$onProcessExit(c); }); - this._innerPid = connect.pid; + try { + var connect = this._ptyNative.connect(this._pty, commandLine, cwd, env, this._useConptyDll, this._windowsAgentJob, function (c) { return _this._$onProcessExit(c); }); + this._innerPid = connect.pid; + } + catch (error) { + // Why: native connect failures happen after JS pipe resources exist; + // stop READY from reconnecting the abandoned output socket during cleanup. + conoutReady.dispose(); + this._inSocket.destroy(); + this._outSocket.destroy(); + this._conoutSocketWorker.dispose(); + throw error; + } } } Object.defineProperty(WindowsPtyAgent.prototype, "inSocket", { @@ -115,6 +135,11 @@ var WindowsPtyAgent = /** @class */ (function () { enumerable: false, configurable: true }); + Object.defineProperty(WindowsPtyAgent.prototype, "windowsAgentJob", { + get: function () { return this._windowsAgentJob; }, + enumerable: false, + configurable: true + }); WindowsPtyAgent.prototype.resize = function (cols, rows) { if (this._useConpty) { if (this._exitCode !== undefined) { @@ -130,6 +155,23 @@ var WindowsPtyAgent = /** @class */ (function () { this._ptyNative.clear(this._pty, this._useConptyDll); } }; + WindowsPtyAgent.prototype.terminateJobTree = function (timeoutMs) { + var _a, _b; + if (!this._windowsAgentJob) { + return undefined; + } + if (this._processExited) { + return Promise.resolve(true); + } + return (_b = (_a = this._ptyNative).terminateJobTree) === null || _b === void 0 ? void 0 : _b.call(_a, this._pty, timeoutMs); + }; + WindowsPtyAgent.prototype.onProcessExit = function (listener) { + if (this._processExited) { + listener(); + return; + } + this._processExitListeners.push(listener); + }; WindowsPtyAgent.prototype.kill = function () { var _this = this; // Tell the agent to kill the pty, this releases handles to the process @@ -154,9 +196,15 @@ var WindowsPtyAgent = /** @class */ (function () { // Close the input write handle to signal the end of session. this._inSocket.destroy(); this._ptyNative.kill(this._pty, this._useConptyDll); - this._outSocket.on('data', function () { + if (this._windowsAgentJob) { + // Why: Job termination can remove every writer before another data event. _this._conoutSocketWorker.dispose(); - }); + } + else { + this._outSocket.on('data', function () { + _this._conoutSocketWorker.dispose(); + }); + } } } else { @@ -221,6 +269,10 @@ var WindowsPtyAgent = /** @class */ (function () { WindowsPtyAgent.prototype._$onProcessExit = function (exitCode) { var _this = this; this._exitCode = exitCode; + this._processExited = true; + var listeners = this._processExitListeners; + this._processExitListeners = []; + listeners.forEach(function (listener) { return listener(); }); if (!this._useConptyDll) { this._flushDataAndCleanUp(); this._outSocket.on('data', function () { return _this._flushDataAndCleanUp(); }); diff --git a/lib/windowsPtyAgent.test.js b/lib/windowsPtyAgent.test.js index 15bbf5ba397ab02a5ec6a11b1c0c6f6ffd137be0..7a894dd2e7c139c657a0ac06d1b25679ee3de89c 100644 --- a/lib/windowsPtyAgent.test.js +++ b/lib/windowsPtyAgent.test.js @@ -10,6 +10,38 @@ function check(file, args, expected) { assert.equal(windowsPtyAgent_1.argsToCommandLine(file, args), expected); } if (process.platform === 'win32') { + describe('Windows agent Job control', function () { + it('reports an already-exited owned Job as drained without another native lookup', function () { + var terminateJobTree = function () { return Promise.resolve(false); }; + var agent = Object.create(windowsPtyAgent_1.WindowsPtyAgent.prototype); + agent._windowsAgentJob = true; + agent._processExited = true; + agent._pty = 1; + agent._ptyNative = { terminateJobTree: terminateJobTree }; + assert.strictEqual(agent.windowsAgentJob, true); + return agent.terminateJobTree(1000).then(function (drained) { + assert.strictEqual(drained, true); + }); + }); + it('does not expose teardown when Job ownership was not established', function () { + var agent = Object.create(windowsPtyAgent_1.WindowsPtyAgent.prototype); + agent._windowsAgentJob = false; + assert.strictEqual(agent.windowsAgentJob, false); + assert.strictEqual(agent.terminateJobTree(1000), undefined); + }); + it('disposes Job output without waiting for a final data event', function () { + var disposed = false; + var agent = Object.create(windowsPtyAgent_1.WindowsPtyAgent.prototype); + agent._windowsAgentJob = true; + agent._useConpty = true; + agent._useConptyDll = true; + agent._inSocket = { destroy: function () { return undefined; } }; + agent._ptyNative = { kill: function () { return undefined; } }; + agent._conoutSocketWorker = { dispose: function () { disposed = true; } }; + agent.kill(); + assert.strictEqual(disposed, true); + }); + }); describe('argsToCommandLine', function () { describe('Plain strings', function () { it('doesn\'t quote plain string', function () { diff --git a/lib/windowsTerminal.js b/lib/windowsTerminal.js index 3c38f89d14cece8081782a9600ebc582b7c2a62a..1cb42dc5d6a78e502673ff84982e8904555c1486 100644 --- a/lib/windowsTerminal.js +++ b/lib/windowsTerminal.js @@ -48,8 +48,23 @@ var WindowsTerminal = /** @class */ (function (_super) { // Functions that need to run after `ready` event is emitted. _this._deferreds = []; // Create new termal. - _this._agent = new windowsPtyAgent_1.WindowsPtyAgent(file, args, parsedEnv, cwd, _this._cols, _this._rows, false, opt.useConpty, opt.useConptyDll, opt.conptyInheritCursor); + _this._agent = new windowsPtyAgent_1.WindowsPtyAgent(file, args, parsedEnv, cwd, _this._cols, _this._rows, false, opt.useConpty, opt.useConptyDll, opt.conptyInheritCursor, opt.windowsAgentJob); _this._socket = _this._agent.outSocket; + var socketClosed = false; + var agentProcessExited = false; + var exitEmitted = false; + var emitExitWhenSafe = function () { + if (exitEmitted || !socketClosed || (_this._agent.windowsAgentJob && !agentProcessExited)) { + return; + } + exitEmitted = true; + _this.emit('exit', _this._agent.exitCode); + _this._close(); + }; + _this._agent.onProcessExit(function () { + agentProcessExited = true; + emitExitWhenSafe(); + }); // Not available until `ready` event emitted. _this._pid = _this._agent.innerPid; _this._fd = _this._agent.fd; @@ -95,8 +110,10 @@ var WindowsTerminal = /** @class */ (function (_super) { }); // Cleanup after the socket is closed. _this._socket.on('close', function () { - _this.emit('exit', _this._agent.exitCode); - _this._close(); + // Why: a Job-owned root can close ConPTY output before the native exit + // callback proves every descendant is gone; public exit needs both facts. + socketClosed = true; + emitExitWhenSafe(); }); }); _this._file = file; @@ -144,6 +161,9 @@ var WindowsTerminal = /** @class */ (function (_super) { _this.kill(); }); }; + WindowsTerminal.prototype.terminateJobTree = function (timeoutMs) { + return this._agent.terminateJobTree(timeoutMs); + }; WindowsTerminal.prototype.kill = function (signal) { var _this = this; this._deferNoArgs(function () { diff --git a/lib/windowsTerminal.test.js b/lib/windowsTerminal.test.js index af5f343d17f012f330fe2292083f297768ff3b5e..cfdabd7b1098e87af102e88b59347692647dadad 100644 --- a/lib/windowsTerminal.test.js +++ b/lib/windowsTerminal.test.js @@ -88,6 +88,14 @@ function pollForProcessTreeSize(pid, size, intervalMs, timeoutMs) { }); } if (process.platform === 'win32') { + describe('WindowsTerminal Job capability fallback', function () { + it('fails closed before a requested Job can fall back to winpty', function () { + assert.throws(function () { return new windowsTerminal_1.WindowsTerminal('cmd.exe', '/C exit', { + useConpty: false, + windowsAgentJob: true + }); }, /Windows agent Job ownership requires ConPTY/); + }); + }); [[false, false], [true, false], [true, true]].forEach(function (_a) { var useConpty = _a[0], useConptyDll = _a[1]; describe("WindowsTerminal (useConpty = " + useConpty + ", useConptyDll = " + useConptyDll + ")", function () { diff --git a/src/conptyConnectFailure.test.ts b/src/conptyConnectFailure.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..c9a657fbcae63e66b5930b7b52aaa98e6e906f24 --- /dev/null +++ b/src/conptyConnectFailure.test.ts @@ -0,0 +1,113 @@ +/** + * Copyright (c) 2026, Stably AI (MIT License). + */ + +import assert = require('assert'); +import { loadNativeModule } from './utils'; +import { WindowsPtyAgent } from './windowsPtyAgent'; + +interface ITestConptyNative { + setTestConnectFailure?(failure: string): void; + getTestResourceState?(): ITestResourceState; +} + +interface ITestResourceState { + ptyCount: number; + closedPseudoConsoleCount: number; + closedPipeHandleCount: number; + closedProcessHandleCount: number; + closedThreadHandleCount: number; + closedJobHandleCount: number; + drainedFailedProcessCount: number; +} + +if (process.platform === 'win32') { + const native = loadNativeModule('conpty').module as ITestConptyNative; + const failureCases = [ + ['assign-job', 'Cannot assign process to Job, error code: 5'], + ['resume-thread', 'Cannot resume process, error code: 6'] + ]; + const environment = Object.keys(process.env) + .filter(key => typeof process.env[key] === 'string') + .map(key => `${key}=${process.env[key]}`); + + const hasTestHooks = (): boolean => + Boolean(native.setTestConnectFailure && native.getTestResourceState); + + const runFailureCase = (failure: string, expectedError: string): void => { + assert.ok(native.setTestConnectFailure && native.getTestResourceState); + const before = native.getTestResourceState(); + native.setTestConnectFailure(failure); + try { + for (let attempt = 1; attempt <= 3; attempt++) { + assert.throws( + () => new WindowsPtyAgent( + process.env.ComSpec || 'cmd.exe', + '/C exit', + environment, + process.cwd(), + 80, + 30, + false, + true, + true, + false, + true + ), + new RegExp(expectedError) + ); + + const resourceState: ITestResourceState = native.getTestResourceState(); + assert.strictEqual(resourceState.ptyCount, before.ptyCount); + assert.strictEqual( + resourceState.closedPseudoConsoleCount, + before.closedPseudoConsoleCount + attempt + ); + assert.strictEqual( + resourceState.closedPipeHandleCount, + before.closedPipeHandleCount + attempt * 2 + ); + assert.strictEqual( + resourceState.closedProcessHandleCount, + before.closedProcessHandleCount + attempt + ); + assert.strictEqual( + resourceState.closedThreadHandleCount, + before.closedThreadHandleCount + attempt + ); + assert.strictEqual( + resourceState.closedJobHandleCount, + before.closedJobHandleCount + attempt + ); + assert.strictEqual( + resourceState.drainedFailedProcessCount, + before.drainedFailedProcessCount + attempt + ); + } + } finally { + native.setTestConnectFailure('none'); + } + }; + + if (typeof describe === 'function') { + describe('ConPTY failed-connect cleanup', () => { + failureCases.forEach(([failure, expectedError]) => { + it(`releases every native PTY resource after ${failure}`, function() { + if (!hasTestHooks()) { + this.skip(); + return; + } + runFailureCase(failure, expectedError); + }); + }); + }); + } else { + assert.ok(hasTestHooks(), 'ConPTY test fault injection was required but is unavailable'); + const leakedResourceWatchdog = setTimeout(() => { + throw new Error('ConPTY failed-connect cleanup left JS resources alive'); + }, 5000); + leakedResourceWatchdog.unref(); + failureCases.forEach(([failure, expectedError]) => runFailureCase(failure, expectedError)); + console.log('ConPTY failed-connect cleanup: 2 passed'); + } +} diff --git a/src/conptyJobTerminationFailure.test.ts b/src/conptyJobTerminationFailure.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..710c34b7e0328c9fbd6f243d0dc45371ea969f62 --- /dev/null +++ b/src/conptyJobTerminationFailure.test.ts @@ -0,0 +1,97 @@ +/** + * Copyright (c) 2026, Stably AI (MIT License). + */ + +import assert = require('assert'); +import { loadNativeModule } from './utils'; +import { WindowsPtyAgent } from './windowsPtyAgent'; + +interface ITestConptyNative { + setTestTerminateJobFailure?(failure: string): void; + getTestResourceState?(): ITestResourceState; +} + +interface ITestResourceState { + closedTerminationJobHandleCount: number; + liveTerminationThreadCount: number; +} + +if (process.platform === 'win32') { + const native = loadNativeModule('conpty').module as ITestConptyNative; + const environment = Object.keys(process.env) + .filter(key => typeof process.env[key] === 'string') + .map(key => `${key}=${process.env[key]}`); + + const createAgent = (): WindowsPtyAgent => new WindowsPtyAgent( + process.env.ComSpec || 'cmd.exe', + '/D /S /C "ping -n 30 127.0.0.1 > nul"', + environment, + process.cwd(), + 80, + 30, + false, + true, + true, + false, + true + ); + + const runFailureCase = (failure: string, expectedError: RegExp): void => { + assert.ok(native.setTestTerminateJobFailure && native.getTestResourceState); + const agent = createAgent(); + const before = native.getTestResourceState(); + native.setTestTerminateJobFailure(failure); + try { + assert.throws(() => agent.terminateJobTree(1000), expectedError); + const after = native.getTestResourceState(); + assert.strictEqual( + after.closedTerminationJobHandleCount, + before.closedTerminationJobHandleCount + 1 + ); + assert.strictEqual(after.liveTerminationThreadCount, before.liveTerminationThreadCount); + } finally { + native.setTestTerminateJobFailure('none'); + agent.kill(); + } + }; + + const runAssertions = (): void => { + assert.ok( + native.setTestTerminateJobFailure && native.getTestResourceState, + 'ConPTY termination test fault injection was required but is unavailable' + ); + assert.throws( + () => new WindowsPtyAgent( + process.env.ComSpec || 'cmd.exe', + '/C exit', + environment, + process.cwd(), + 80, + 30, + false, + false, + false, + false, + true + ), + /Windows agent Job ownership requires ConPTY/ + ); + runFailureCase('thread-allocation', /Injected termination thread allocation failure/); + runFailureCase('threadsafe-function', /Injected termination ThreadSafeFunction failure/); + }; + + if (typeof describe === 'function') { + describe('ConPTY Job capability and termination cleanup', () => { + it('fails closed without ConPTY and releases allocation-failure resources', () => { + runAssertions(); + }); + }); + } else { + const leakedWorkerWatchdog = setTimeout(() => { + throw new Error('ConPTY Job termination cleanup left JS resources alive'); + }, 5000); + leakedWorkerWatchdog.unref(); + runAssertions(); + console.log('ConPTY Job capability and termination cleanup: 3 passed'); + } +} diff --git a/src/conpty_console_list_agent.ts b/src/conpty_console_list_agent.ts index 181ccabbbe9c4948a9725fb1db907a68e9de01fc..67f31facf85562b67adbfbd04ce28ddd8eeb4a79 100644 --- a/src/conpty_console_list_agent.ts +++ b/src/conpty_console_list_agent.ts @@ -10,6 +10,12 @@ 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[]; +try { + consoleProcessList = getConsoleProcessList(shellPid); +} catch { + // Why: AttachConsole can fail after the shell exits; parent already has this fallback. + consoleProcessList = [shellPid]; +} process.send!({ consoleProcessList }); process.exit(0); diff --git a/src/interfaces.ts b/src/interfaces.ts index a269e77bd2acf31c762b0fd8e13e6bf435fd7d07..adc2e8d10190de6bbeea1f2b53320ef92e0b2935 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -121,6 +121,8 @@ export interface IWindowsPtyForkOptions extends IBasePtyForkOptions { useConpty?: boolean; useConptyDll?: boolean; conptyInheritCursor?: boolean; + /** Orca-specific: own this ConPTY and all descendants in a kill-on-close Job. */ + windowsAgentJob?: boolean; } export interface IPtyOpenOptions { diff --git a/src/native.d.ts b/src/native.d.ts index c53e086b4968d6fb0bf601ace3a813809433fedb..ce2d197bbf618f17cdcbf1e6127e831715a8beb3 100644 --- a/src/native.d.ts +++ b/src/native.d.ts @@ -4,10 +4,11 @@ interface IConptyNative { startProcess(file: string, cols: number, rows: number, debug: boolean, pipeName: string, conptyInheritCursor: boolean, useConptyDll: boolean): IConptyProcess; - connect(ptyId: number, commandLine: string, cwd: string, env: string[], useConptyDll: boolean, onExitCallback: (exitCode: number) => void): { pid: number }; + connect(ptyId: number, commandLine: string, cwd: string, env: string[], useConptyDll: boolean, windowsAgentJob: boolean, onExitCallback: (exitCode: number) => void): { pid: number }; resize(ptyId: number, cols: number, rows: number, useConptyDll: boolean): void; clear(ptyId: number, useConptyDll: boolean): void; kill(ptyId: number, useConptyDll: boolean): void; + terminateJobTree?(ptyId: number, timeoutMs: number): Promise | undefined; } interface IWinptyNative { diff --git a/src/unix/pty.cc b/src/unix/pty.cc index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..61f39f0cbb91faa2c515f35d2ca850564e6368d9 100644 --- a/src/unix/pty.cc +++ b/src/unix/pty.cc @@ -23,7 +23,9 @@ #include #include #include +#include #include +#include #include #include @@ -237,13 +239,23 @@ pty_getproc(int, char *); #endif #if defined(__APPLE__) || defined(__OpenBSD__) +struct pty_spawn_error { + const char* step; + int errnum; + std::string detail_name; + std::string detail_value; +}; + +static std::string +pty_format_spawn_error(const pty_spawn_error&); + static void pty_posix_spawn(char** argv, char** env, const struct termios *termp, const struct winsize *winp, int* master, pid_t* pid, - int* err); + pty_spawn_error* err); #endif struct DelBuf { @@ -367,10 +379,11 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) { argv[i + 3] = strdup(arg.c_str()); } - int err = -1; - pty_posix_spawn(argv, env, term, &winp, &master, &pid, &err); - if (err != 0) { - throw Napi::Error::New(napiEnv, "posix_spawnp failed."); + pty_spawn_error spawn_error = { NULL, 0, "", "" }; + pty_posix_spawn(argv, env, term, &winp, &master, &pid, &spawn_error); + if (spawn_error.errnum != 0) { + std::string spawn_message = pty_format_spawn_error(spawn_error); + throw Napi::Error::New(napiEnv, spawn_message); } if (pty_nonblock(master) == -1) { throw Napi::Error::New(napiEnv, "Could not set master fd to nonblocking."); @@ -684,15 +697,73 @@ pty_getproc(int fd, char *tty) { #endif #if defined(__APPLE__) +static const char* +pty_errno_name(int errnum) { + switch (errnum) { + case E2BIG: return "E2BIG"; + case EACCES: return "EACCES"; + case EAGAIN: return "EAGAIN"; + case EMFILE: return "EMFILE"; + case ENFILE: return "ENFILE"; + case ENOENT: return "ENOENT"; + case ENOMEM: return "ENOMEM"; + default: return "errno"; + } +} + +static void +pty_set_spawn_error(pty_spawn_error* err, + const char* step, + int errnum, + const char* detail_name = NULL, + const char* detail_value = NULL) { + err->step = step; + err->errnum = errnum; + err->detail_name = detail_name ? detail_name : ""; + err->detail_value = detail_value ? detail_value : ""; +} + +static std::string +pty_format_spawn_error(const pty_spawn_error& err) { + char errno_buf[64]; + snprintf(errno_buf, sizeof(errno_buf), "%d", err.errnum); + + std::string message = "node-pty: "; + message += err.step ? err.step : "unknown"; + message += " failed: "; + message += pty_errno_name(err.errnum); + message += " (errno "; + message += errno_buf; + message += ", "; + message += strerror(err.errnum); + message += ")"; + + if (!err.detail_name.empty()) { + message += " - "; + message += err.detail_name; + message += "='"; + message += err.detail_value; + message += "'"; + } + + return message; +} + static void pty_posix_spawn(char** argv, char** env, const struct termios *termp, const struct winsize *winp, int* master, pid_t* pid, - int* err) { - int low_fds[3]; + pty_spawn_error* err) { + int low_fds[3] = {-1, -1, -1}; size_t count = 0; + int res = -1; + int slave = -1; + posix_spawn_file_actions_t acts; + bool acts_initialized = false; + posix_spawnattr_t attrs; + bool attrs_initialized = false; for (; count < 3; count++) { low_fds[count] = posix_openpt(O_RDWR); @@ -706,80 +777,118 @@ pty_posix_spawn(char** argv, char** env, POSIX_SPAWN_SETSID; *master = posix_openpt(O_RDWR); if (*master == -1) { - return; + pty_set_spawn_error(err, "posix_openpt", errno); + goto done; + } + + res = grantpt(*master); + if (res == -1) { + pty_set_spawn_error(err, "grantpt", errno); + goto done; } - int res = grantpt(*master) || unlockpt(*master); + res = unlockpt(*master); if (res == -1) { - return; + pty_set_spawn_error(err, "unlockpt", errno); + goto done; } // Use TIOCPTYGNAME instead of ptsname() to avoid threading problems. - int slave; char slave_pty_name[128]; res = ioctl(*master, TIOCPTYGNAME, slave_pty_name); if (res == -1) { - return; + pty_set_spawn_error(err, "ioctl_TIOCPTYGNAME", errno); + goto done; } slave = open(slave_pty_name, O_RDWR | O_NOCTTY); if (slave == -1) { - return; + pty_set_spawn_error(err, "open_slave", errno, "slave", slave_pty_name); + goto done; } if (termp) { res = tcsetattr(slave, TCSANOW, termp); if (res == -1) { - return; + pty_set_spawn_error(err, "tcsetattr", errno, "slave", slave_pty_name); + goto done; }; } if (winp) { res = ioctl(slave, TIOCSWINSZ, winp); if (res == -1) { - return; + pty_set_spawn_error(err, "ioctl_TIOCSWINSZ", errno, "slave", slave_pty_name); + goto done; } } - posix_spawn_file_actions_t acts; - posix_spawn_file_actions_init(&acts); + res = posix_spawn_file_actions_init(&acts); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawn_file_actions_init", res); + goto done; + } + acts_initialized = true; posix_spawn_file_actions_adddup2(&acts, slave, STDIN_FILENO); posix_spawn_file_actions_adddup2(&acts, slave, STDOUT_FILENO); posix_spawn_file_actions_adddup2(&acts, slave, STDERR_FILENO); posix_spawn_file_actions_addclose(&acts, slave); posix_spawn_file_actions_addclose(&acts, *master); - posix_spawnattr_t attrs; - posix_spawnattr_init(&attrs); - *err = posix_spawnattr_setflags(&attrs, flags); - if (*err != 0) { + res = posix_spawnattr_init(&attrs); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawnattr_init", res); + goto done; + } + attrs_initialized = true; + res = posix_spawnattr_setflags(&attrs, flags); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawnattr_setflags", res); goto done; } sigset_t signal_set; /* Reset all signal the child to their default behavior */ sigfillset(&signal_set); - *err = posix_spawnattr_setsigdefault(&attrs, &signal_set); - if (*err != 0) { + res = posix_spawnattr_setsigdefault(&attrs, &signal_set); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawnattr_setsigdefault", res); goto done; } /* Reset the signal mask for all signals */ sigemptyset(&signal_set); - *err = posix_spawnattr_setsigmask(&attrs, &signal_set); - if (*err != 0) { + res = posix_spawnattr_setsigmask(&attrs, &signal_set); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawnattr_setsigmask", res); goto done; } do - *err = posix_spawn(pid, argv[0], &acts, &attrs, argv, env); - while (*err == EINTR); + res = posix_spawn(pid, argv[0], &acts, &attrs, argv, env); + while (res == EINTR); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawn", res, "helper", argv[0]); + } done: - posix_spawn_file_actions_destroy(&acts); - posix_spawnattr_destroy(&attrs); + if (acts_initialized) { + posix_spawn_file_actions_destroy(&acts); + } + if (attrs_initialized) { + posix_spawnattr_destroy(&attrs); + } + if (slave != -1) { + close(slave); + } + if (err->errnum != 0 && *master != -1) { + close(*master); + *master = -1; + } - for (; count > 0; count--) { - close(low_fds[count]); + for (size_t i = 0; i <= count && i < 3; i++) { + if (low_fds[i] != -1) { + close(low_fds[i]); + } } } #endif diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 7b286d3d644c26141df516929703aa6e129df4b2..cf1570b820a01284392f50dbfa5733bd833f992d 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -16,6 +16,7 @@ #include // PathCombine, PathIsRelative #include #include +#include #include #include #include @@ -45,12 +46,40 @@ struct pty_baton { HPCON hpc; HANDLE hShell; - - pty_baton(int _id, HANDLE _hIn, HANDLE _hOut, HPCON _hpc) : id(_id), hIn(_hIn), hOut(_hOut), hpc(_hpc) {}; + HANDLE hJob = nullptr; + // Why: failed connect cleanup must close HPCON without a second DLL load + // that can itself throw and strand the original resources. + PFNCLOSEPSEUDOCONSOLE closePseudoConsole; + PFNRELEASEPSEUDOCONSOLE releasePseudoConsole; + + pty_baton( + int _id, + HANDLE _hIn, + HANDLE _hOut, + HPCON _hpc, + PFNCLOSEPSEUDOCONSOLE _closePseudoConsole, + PFNRELEASEPSEUDOCONSOLE _releasePseudoConsole) : + id(_id), + hIn(_hIn), + hOut(_hOut), + hpc(_hpc), + closePseudoConsole(_closePseudoConsole), + releasePseudoConsole(_releasePseudoConsole) {}; }; static std::vector> ptyHandles; static volatile LONG ptyCounter; +#ifdef NODE_PTY_TEST_FAULTS +static LONG closedPseudoConsoleCount = 0; +static LONG closedPipeHandleCount = 0; +static LONG closedProcessHandleCount = 0; +static LONG closedThreadHandleCount = 0; +static LONG closedJobHandleCount = 0; +static LONG drainedFailedProcessCount = 0; +static LONG closedTerminationJobHandleCount = 0; +static LONG allocatedTerminationThreadCount = 0; +static LONG releasedTerminationThreadCount = 0; +#endif static pty_baton* get_pty_baton(int id) { auto it = std::find_if(ptyHandles.begin(), ptyHandles.end(), [id](const auto& ptyHandle) { @@ -73,6 +102,147 @@ static bool remove_pty_baton(int id) { return false; } +static void close_pty_resources(pty_baton* handle) { + if (handle->hpc != nullptr) { + handle->closePseudoConsole(handle->hpc); + handle->hpc = nullptr; +#ifdef NODE_PTY_TEST_FAULTS + InterlockedIncrement(&closedPseudoConsoleCount); +#endif + } + if (handle->hIn != INVALID_HANDLE_VALUE) { + CloseHandle(handle->hIn); + handle->hIn = INVALID_HANDLE_VALUE; +#ifdef NODE_PTY_TEST_FAULTS + InterlockedIncrement(&closedPipeHandleCount); +#endif + } + if (handle->hOut != INVALID_HANDLE_VALUE) { + CloseHandle(handle->hOut); + handle->hOut = INVALID_HANDLE_VALUE; +#ifdef NODE_PTY_TEST_FAULTS + InterlockedIncrement(&closedPipeHandleCount); +#endif + } +} + +static void cleanup_failed_connect( + pty_baton* handle, + PROCESS_INFORMATION* process, + PPROC_THREAD_ATTRIBUTE_LIST attributeList) { + if (process->hProcess != nullptr) { + TerminateProcess(process->hProcess, 1); + const DWORD waitResult = WaitForSingleObject(process->hProcess, INFINITE); + CloseHandle(process->hProcess); + process->hProcess = nullptr; +#ifdef NODE_PTY_TEST_FAULTS + InterlockedIncrement(&closedProcessHandleCount); + if (waitResult == WAIT_OBJECT_0) { + InterlockedIncrement(&drainedFailedProcessCount); + } +#else + (void)waitResult; +#endif + } + if (process->hThread != nullptr) { + CloseHandle(process->hThread); + process->hThread = nullptr; +#ifdef NODE_PTY_TEST_FAULTS + InterlockedIncrement(&closedThreadHandleCount); +#endif + } + if (handle->hJob != nullptr) { + CloseHandle(handle->hJob); + handle->hJob = nullptr; +#ifdef NODE_PTY_TEST_FAULTS + InterlockedIncrement(&closedJobHandleCount); +#endif + } + if (attributeList != nullptr) { + DeleteProcThreadAttributeList(attributeList); + } + close_pty_resources(handle); + const int id = handle->id; + assert(remove_pty_baton(id)); +} + +#ifdef NODE_PTY_TEST_FAULTS +// Why: CI must exercise Win32 failure branches that cannot be triggered +// deterministically through the production API. +enum class ConnectFailurePoint { + None, + AssignJob, + ResumeThread +}; + +enum class TerminateJobFailurePoint { + None, + ThreadAllocation, + ThreadSafeFunctionCreation +}; + +static ConnectFailurePoint connectFailurePoint = ConnectFailurePoint::None; +static TerminateJobFailurePoint terminateJobFailurePoint = TerminateJobFailurePoint::None; + +static Napi::Value PtySetTestConnectFailure(const Napi::CallbackInfo& info) { + if (info.Length() != 1 || !info[0].IsString()) { + throw Napi::Error::New(info.Env(), "Usage: pty.setTestConnectFailure(failure)"); + } + const std::string failure = info[0].As().Utf8Value(); + if (failure == "assign-job") { + connectFailurePoint = ConnectFailurePoint::AssignJob; + } else if (failure == "resume-thread") { + connectFailurePoint = ConnectFailurePoint::ResumeThread; + } else { + connectFailurePoint = ConnectFailurePoint::None; + } + return info.Env().Undefined(); +} + +static Napi::Value PtySetTestTerminateJobFailure(const Napi::CallbackInfo& info) { + if (info.Length() != 1 || !info[0].IsString()) { + throw Napi::Error::New(info.Env(), "Usage: pty.setTestTerminateJobFailure(failure)"); + } + const std::string failure = info[0].As().Utf8Value(); + if (failure == "thread-allocation") { + terminateJobFailurePoint = TerminateJobFailurePoint::ThreadAllocation; + } else if (failure == "threadsafe-function") { + terminateJobFailurePoint = TerminateJobFailurePoint::ThreadSafeFunctionCreation; + } else { + terminateJobFailurePoint = TerminateJobFailurePoint::None; + } + return info.Env().Undefined(); +} + +static Napi::Value PtyGetTestResourceState(const Napi::CallbackInfo& info) { + auto result = Napi::Object::New(info.Env()); + result.Set("ptyCount", Napi::Number::New(info.Env(), ptyHandles.size())); + result.Set( + "closedPseudoConsoleCount", + Napi::Number::New(info.Env(), closedPseudoConsoleCount)); + result.Set("closedPipeHandleCount", Napi::Number::New(info.Env(), closedPipeHandleCount)); + result.Set( + "closedProcessHandleCount", + Napi::Number::New(info.Env(), closedProcessHandleCount)); + result.Set( + "closedThreadHandleCount", + Napi::Number::New(info.Env(), closedThreadHandleCount)); + result.Set("closedJobHandleCount", Napi::Number::New(info.Env(), closedJobHandleCount)); + result.Set( + "drainedFailedProcessCount", + Napi::Number::New(info.Env(), drainedFailedProcessCount)); + result.Set( + "closedTerminationJobHandleCount", + Napi::Number::New(info.Env(), closedTerminationJobHandleCount)); + result.Set( + "liveTerminationThreadCount", + Napi::Number::New( + info.Env(), + allocatedTerminationThreadCount - releasedTerminationThreadCount)); + return result; +} +#endif + struct ExitEvent { int exit_code = 0; }; @@ -91,7 +261,18 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) { delete th; }); *th = std::thread([tsfn = std::move(tsfn), baton] { - auto callback = [](Napi::Env env, Napi::Function cb, ExitEvent *exit_event) { + auto callback = [baton](Napi::Env env, Napi::Function cb, ExitEvent *exit_event) { + if (baton->hShell != nullptr) { + GetExitCodeProcess(baton->hShell, (LPDWORD)(&exit_event->exit_code)); + CloseHandle(baton->hShell); + baton->hShell = nullptr; + } + HANDLE hJob = baton->hJob; + baton->hJob = nullptr; + assert(remove_pty_baton(baton->id)); + if (hJob != nullptr) { + CloseHandle(hJob); + } cb.Call({Napi::Number::New(env, exit_event->exit_code)}); delete exit_event; }; @@ -99,15 +280,41 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) { ExitEvent *exit_event = new ExitEvent; // Wait for process to complete. WaitForSingleObject(baton->hShell, INFINITE); - // Get process exit code. - GetExitCodeProcess(baton->hShell, (LPDWORD)(&exit_event->exit_code)); - // Clean up handles - CloseHandle(baton->hShell); - assert(remove_pty_baton(baton->id)); + + if (baton->hJob != nullptr) { + // Why: an agent root can exit before its detached tools. Do not publish + // PTY exit until Windows proves every member has released its handles. + TerminateJobObject(baton->hJob, 1); + while (true) { + JOBOBJECT_BASIC_ACCOUNTING_INFORMATION accounting{}; + if (QueryInformationJobObject( + baton->hJob, + JobObjectBasicAccountingInformation, + &accounting, + sizeof(accounting), + nullptr) && accounting.ActiveProcesses == 0) { + break; + } + // A permanent query/termination failure deliberately keeps the PTY + // owned and un-reaped so destructive callers cannot race deletion. + Sleep(25); + } + } auto status = tsfn.BlockingCall(exit_event, callback); // In main thread switch (status) { case napi_closing: + // The environment no longer accepts JS calls, so no native method can + // race these per-baton handles during finalization. + if (baton->hShell != nullptr) { + CloseHandle(baton->hShell); + baton->hShell = nullptr; + } + if (baton->hJob != nullptr) { + CloseHandle(baton->hJob); + baton->hJob = nullptr; + } + delete exit_event; break; case napi_queue_full: @@ -194,6 +401,8 @@ HRESULT CreateNamedPipesAndPseudoConsole(const Napi::CallbackInfo& info, HANDLE *phInput, HANDLE *phOutput, HPCON* phPC, + PFNCLOSEPSEUDOCONSOLE* pfnClose, + PFNRELEASEPSEUDOCONSOLE* pfnRelease, std::wstring& inName, std::wstring& outName, const std::wstring& pipeName, @@ -214,6 +423,18 @@ HRESULT CreateNamedPipesAndPseudoConsole(const Napi::CallbackInfo& info, return E_INVALIDARG; } + *pfnClose = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress( + (HMODULE)hLibrary, + useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole"); + *pfnRelease = useConptyDll + ? (PFNRELEASEPSEUDOCONSOLE)GetProcAddress( + (HMODULE)hLibrary, + "ConptyReleasePseudoConsole") + : nullptr; + if (*pfnClose == nullptr) { + return HRESULT_FROM_WIN32(GetLastError()); + } + bool success = createDataServerPipe(true, L"in", phInput, inName, pipeName); if (!success) { @@ -289,7 +510,21 @@ static Napi::Value PtyStartProcess(const Napi::CallbackInfo& info) { HANDLE hIn, hOut; HPCON hpc; - HRESULT hr = CreateNamedPipesAndPseudoConsole(info, {cols, rows}, inheritCursor ? 1/*PSEUDOCONSOLE_INHERIT_CURSOR*/ : 0, &hIn, &hOut, &hpc, inName, outName, pipeName, useConptyDll); + PFNCLOSEPSEUDOCONSOLE closePseudoConsole; + PFNRELEASEPSEUDOCONSOLE releasePseudoConsole; + HRESULT hr = CreateNamedPipesAndPseudoConsole( + info, + {cols, rows}, + inheritCursor ? 1/*PSEUDOCONSOLE_INHERIT_CURSOR*/ : 0, + &hIn, + &hOut, + &hpc, + &closePseudoConsole, + &releasePseudoConsole, + inName, + outName, + pipeName, + useConptyDll); // Restore default handling of ctrl+c SetConsoleCtrlHandler(NULL, FALSE); @@ -302,7 +537,13 @@ static Napi::Value PtyStartProcess(const Napi::CallbackInfo& info) { const int ptyId = InterlockedIncrement(&ptyCounter); marshal.Set("pty", Napi::Number::New(env, ptyId)); ptyHandles.emplace_back( - std::make_unique(ptyId, hIn, hOut, hpc)); + std::make_unique( + ptyId, + hIn, + hOut, + hpc, + closePseudoConsole, + releasePseudoConsole)); } else { throw Napi::Error::New(env, "Cannot launch conpty"); } @@ -333,14 +574,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { std::stringstream errorText; BOOL fSuccess = FALSE; - if (info.Length() != 6 || + if (info.Length() != 7 || !info[0].IsNumber() || !info[1].IsString() || !info[2].IsString() || !info[3].IsArray() || !info[4].IsBoolean() || - !info[5].IsFunction()) { - throw Napi::Error::New(env, "Usage: pty.connect(id, cmdline, cwd, env, useConptyDll, exitCallback)"); + !info[5].IsBoolean() || + !info[6].IsFunction()) { + throw Napi::Error::New(env, "Usage: pty.connect(id, cmdline, cwd, env, useConptyDll, windowsAgentJob, exitCallback)"); } const int id = info[0].As().Int32Value(); @@ -348,7 +590,8 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { const std::wstring cwd(path_util::to_wstring(info[2].As())); const Napi::Array envValues = info[3].As(); const bool useConptyDll = info[4].As().Value(); - Napi::Function exitCallback = info[5].As(); + const bool windowsAgentJob = info[5].As().Value(); + Napi::Function exitCallback = info[6].As(); // Fetch pty handle from ID and start process pty_baton* handle = get_pty_baton(id); @@ -391,11 +634,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { SIZE_T size = 0; InitializeProcThreadAttributeList(NULL, 1, 0, &size); - BYTE *attrList = new BYTE[size]; - siEx.lpAttributeList = reinterpret_cast(attrList); + auto attrList = std::make_unique(size); + siEx.lpAttributeList = reinterpret_cast(attrList.get()); + PROCESS_INFORMATION piClient{}; fSuccess = InitializeProcThreadAttributeList(siEx.lpAttributeList, 1, 0, &size); if (!fSuccess) { + const DWORD attributeError = GetLastError(); + cleanup_failed_connect(handle, &piClient, nullptr); + SetLastError(attributeError); throw errorWithCode(info, "InitializeProcThreadAttributeList failed"); } fSuccess = UpdateProcThreadAttribute(siEx.lpAttributeList, @@ -406,38 +653,82 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { NULL, NULL); if (!fSuccess) { + const DWORD attributeError = GetLastError(); + cleanup_failed_connect(handle, &piClient, siEx.lpAttributeList); + SetLastError(attributeError); throw errorWithCode(info, "UpdateProcThreadAttribute failed"); } - PROCESS_INFORMATION piClient{}; fSuccess = !!CreateProcessW( nullptr, mutableCommandline.get(), nullptr, // lpProcessAttributes nullptr, // lpThreadAttributes false, // bInheritHandles VERY IMPORTANT that this is false - EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT, // dwCreationFlags + EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT | + (windowsAgentJob ? CREATE_SUSPENDED : 0), // dwCreationFlags envArg, // lpEnvironment mutableCwd.get(), // lpCurrentDirectory &siEx.StartupInfo, // lpStartupInfo &piClient // lpProcessInformation ); if (!fSuccess) { + const DWORD createError = GetLastError(); + cleanup_failed_connect(handle, &piClient, siEx.lpAttributeList); + SetLastError(createError); throw errorWithCode(info, "Cannot create process"); } - HANDLE hLibrary = LoadConptyDll(info, useConptyDll); - bool fLoadedDll = hLibrary != nullptr; - if (useConptyDll && fLoadedDll) - { - PFNRELEASEPSEUDOCONSOLE const pfnReleasePseudoConsole = (PFNRELEASEPSEUDOCONSOLE)GetProcAddress( - (HMODULE)hLibrary, "ConptyReleasePseudoConsole"); - if (pfnReleasePseudoConsole) - { - pfnReleasePseudoConsole(handle->hpc); + if (windowsAgentJob) { + // Job setup is atomic while the shell is suspended; partial ownership is unsafe. + HANDLE hJob = CreateJobObjectW(nullptr, nullptr); + JOBOBJECT_EXTENDED_LIMIT_INFORMATION limits{}; + limits.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + bool jobReady = hJob != nullptr && + SetInformationJobObject(hJob, JobObjectExtendedLimitInformation, &limits, sizeof(limits)); +#ifdef NODE_PTY_TEST_FAULTS + if (jobReady && connectFailurePoint == ConnectFailurePoint::AssignJob) { + SetLastError(ERROR_ACCESS_DENIED); + jobReady = false; + } +#endif + if (jobReady) { + jobReady = AssignProcessToJobObject(hJob, piClient.hProcess); + } + if (!jobReady) { + const DWORD jobError = GetLastError(); + handle->hJob = hJob; + cleanup_failed_connect(handle, &piClient, siEx.lpAttributeList); + SetLastError(jobError); + throw errorWithCode(info, "Cannot assign process to Job"); + } + handle->hJob = hJob; + + DWORD resumeResult; +#ifdef NODE_PTY_TEST_FAULTS + if (connectFailurePoint == ConnectFailurePoint::ResumeThread) { + SetLastError(ERROR_INVALID_HANDLE); + resumeResult = static_cast(-1); + } else { + resumeResult = ResumeThread(piClient.hThread); + } +#else + resumeResult = ResumeThread(piClient.hThread); +#endif + if (resumeResult == static_cast(-1)) { + const DWORD resumeError = GetLastError(); + cleanup_failed_connect(handle, &piClient, siEx.lpAttributeList); + SetLastError(resumeError); + throw errorWithCode(info, "Cannot resume process"); } } + DeleteProcThreadAttributeList(siEx.lpAttributeList); + + if (handle->releasePseudoConsole != nullptr) { + handle->releasePseudoConsole(handle->hpc); + } + // Update handle handle->hShell = piClient.hProcess; @@ -445,7 +736,9 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { CloseHandle(piClient.hThread); // Close the input read and output write handle of the pseudoconsole CloseHandle(handle->hIn); + handle->hIn = INVALID_HANDLE_VALUE; CloseHandle(handle->hOut); + handle->hOut = INVALID_HANDLE_VALUE; SetupExitCallback(env, exitCallback, handle); @@ -544,21 +837,10 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) { int id = info[0].As().Int32Value(); const bool useConptyDll = info[1].As().Value(); - const pty_baton* handle = get_pty_baton(id); + pty_baton* handle = get_pty_baton(id); if (handle != nullptr) { - HANDLE hLibrary = LoadConptyDll(info, useConptyDll); - bool fLoadedDll = hLibrary != nullptr; - if (fLoadedDll) - { - PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress( - (HMODULE)hLibrary, - useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole"); - if (pfnClosePseudoConsole) - { - pfnClosePseudoConsole(handle->hpc); - } - } + close_pty_resources(handle); if (useConptyDll) { TerminateProcess(handle->hShell, 1); } @@ -567,6 +849,178 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) { return env.Undefined(); } +struct JobTerminationResult { + bool drained = false; +}; + +struct JobTerminationWorker { + std::thread thread; + JobTerminationResult result; +}; + +static void close_termination_job_handle(HANDLE handle) { + if (handle == nullptr) { + return; + } + CloseHandle(handle); +#ifdef NODE_PTY_TEST_FAULTS + InterlockedIncrement(&closedTerminationJobHandleCount); +#endif +} + +class ScopedTerminationJobHandle { + public: + explicit ScopedTerminationJobHandle(HANDLE handle) : handle(handle) {} + ~ScopedTerminationJobHandle() { + close_termination_job_handle(handle); + } + ScopedTerminationJobHandle(const ScopedTerminationJobHandle&) = delete; + ScopedTerminationJobHandle& operator=(const ScopedTerminationJobHandle&) = delete; + HANDLE get() const { return handle; } + HANDLE release() { + const HANDLE released = handle; + handle = nullptr; + return released; + } + + private: + HANDLE handle; +}; + +static void release_termination_worker(JobTerminationWorker* worker) { + if (worker->thread.joinable()) { + worker->thread.join(); + } + delete worker; +#ifdef NODE_PTY_TEST_FAULTS + InterlockedIncrement(&releasedTerminationThreadCount); +#endif +} + +static Napi::Value PtyTerminateJobTree(const Napi::CallbackInfo& info) { + Napi::Env env(info.Env()); + Napi::HandleScope scope(env); + + if (info.Length() != 2 || !info[0].IsNumber() || !info[1].IsNumber()) { + throw Napi::Error::New(env, "Usage: pty.terminateJobTree(id, timeoutMs)"); + } + + const int id = info[0].As().Int32Value(); + const pty_baton* handle = get_pty_baton(id); + if (handle == nullptr || handle->hJob == nullptr) { + return env.Undefined(); + } + + const double timeoutValue = info[1].As().DoubleValue(); + if (timeoutValue < 1 || timeoutValue > 60000) { + throw Napi::RangeError::New(env, "timeoutMs must be between 1 and 60000"); + } + + HANDLE hJob = nullptr; + if (!DuplicateHandle( + GetCurrentProcess(), + handle->hJob, + GetCurrentProcess(), + &hJob, + 0, + FALSE, + DUPLICATE_SAME_ACCESS)) { + return env.Undefined(); + } + ScopedTerminationJobHandle jobHandle(hJob); + if (!TerminateJobObject(jobHandle.get(), 1)) { + return env.Undefined(); + } + + // Why: termination is asynchronous. Keep a duplicate Job handle until the + // OS reports no members instead of treating the root process exit as proof. + auto deferred = Napi::Promise::Deferred::New(env); +#ifdef NODE_PTY_TEST_FAULTS + if (terminateJobFailurePoint == TerminateJobFailurePoint::ThreadAllocation) { + throw Napi::Error::New(env, "Injected termination thread allocation failure"); + } +#endif + auto worker = std::unique_ptr( + new JobTerminationWorker, + release_termination_worker); +#ifdef NODE_PTY_TEST_FAULTS + InterlockedIncrement(&allocatedTerminationThreadCount); + if (terminateJobFailurePoint == TerminateJobFailurePoint::ThreadSafeFunctionCreation) { + throw Napi::Error::New(env, "Injected termination ThreadSafeFunction failure"); + } +#endif + JobTerminationWorker *workerContext = worker.get(); + auto tsfn = Napi::ThreadSafeFunction::New( + env, + Napi::Function::New(env, [](const Napi::CallbackInfo&) {}), + "PtyTerminateJobTree_resource", + 0, + 1, + [workerContext](Napi::Env) { release_termination_worker(workerContext); }); + const napi_threadsafe_function rawTsfn = tsfn; + const DWORD timeoutMs = static_cast(timeoutValue); + try { + workerContext->thread = std::thread([ + rawTsfn, + deferred, + workerJob = jobHandle.get(), + workerContext, + timeoutMs] { + Napi::ThreadSafeFunction workerTsfn(rawTsfn); + ScopedTerminationJobHandle workerJobHandle(workerJob); + const ULONGLONG startedAt = GetTickCount64(); + while (true) { + JOBOBJECT_BASIC_ACCOUNTING_INFORMATION accounting{}; + if (!QueryInformationJobObject( + workerJobHandle.get(), + JobObjectBasicAccountingInformation, + &accounting, + sizeof(accounting), + nullptr)) { + break; + } + if (accounting.ActiveProcesses == 0) { + workerContext->result.drained = true; + break; + } + if (GetTickCount64() - startedAt >= timeoutMs) { + break; + } + Sleep(25); + } + const auto status = workerTsfn.BlockingCall( + &workerContext->result, + [deferred](Napi::Env env, Napi::Function, JobTerminationResult *result) { + deferred.Resolve(Napi::Boolean::New(env, result->drained)); + }); + switch (status) { + case napi_closing: + break; + case napi_queue_full: + Napi::Error::Fatal("PtyTerminateJobTree", "Queue was full"); + case napi_ok: + if (workerTsfn.Release() != napi_ok) { + Napi::Error::Fatal("PtyTerminateJobTree", "ThreadSafeFunction.Release() failed"); + } + break; + default: + Napi::Error::Fatal("PtyTerminateJobTree", "ThreadSafeFunction.BlockingCall() failed"); + } + }); + } catch (...) { + // Why: once TSFN creation succeeds, its finalizer owns the thread allocation. + worker.release(); + if (tsfn.Release() != napi_ok) { + Napi::Error::Fatal("PtyTerminateJobTree", "ThreadSafeFunction.Release() failed"); + } + throw; + } + jobHandle.release(); + worker.release(); + + return deferred.Promise(); +} + /** * Init */ @@ -577,6 +1031,14 @@ Napi::Object init(Napi::Env env, Napi::Object exports) { exports.Set("resize", Napi::Function::New(env, PtyResize)); exports.Set("clear", Napi::Function::New(env, PtyClear)); exports.Set("kill", Napi::Function::New(env, PtyKill)); + exports.Set("terminateJobTree", Napi::Function::New(env, PtyTerminateJobTree)); +#ifdef NODE_PTY_TEST_FAULTS + exports.Set("setTestConnectFailure", Napi::Function::New(env, PtySetTestConnectFailure)); + exports.Set( + "setTestTerminateJobFailure", + Napi::Function::New(env, PtySetTestTerminateJobFailure)); + exports.Set("getTestResourceState", Napi::Function::New(env, PtyGetTestResourceState)); +#endif return exports; }; diff --git a/src/windowsPtyAgent.test.ts b/src/windowsPtyAgent.test.ts index dc2104b34fd396722a9e30821ff0b7dea18b4295..ca448e7ba0ed9bcee0081c15b40f048021becd0d 100644 --- a/src/windowsPtyAgent.test.ts +++ b/src/windowsPtyAgent.test.ts @@ -4,13 +4,52 @@ */ 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); } if (process.platform === 'win32') { + describe('Windows agent Job control', () => { + it('reports an already-exited owned Job as drained without another native lookup', () => { + const terminateJobTree = () => Promise.resolve(false); + const agent = Object.create(WindowsPtyAgent.prototype) as any; + agent._windowsAgentJob = true; + agent._processExited = true; + agent._pty = 1; + agent._ptyNative = { terminateJobTree }; + + assert.strictEqual(agent.windowsAgentJob, true); + return agent.terminateJobTree(1000).then((drained: boolean) => { + assert.strictEqual(drained, true); + }); + }); + + it('does not expose teardown when Job ownership was not established', () => { + const agent = Object.create(WindowsPtyAgent.prototype) as any; + agent._windowsAgentJob = false; + + assert.strictEqual(agent.windowsAgentJob, false); + assert.strictEqual(agent.terminateJobTree(1000), undefined); + }); + + it('disposes Job output without waiting for a final data event', () => { + let disposed = false; + const agent = Object.create(WindowsPtyAgent.prototype) as any; + agent._windowsAgentJob = true; + agent._useConpty = true; + agent._useConptyDll = true; + agent._inSocket = { destroy: () => undefined }; + agent._ptyNative = { kill: () => undefined }; + agent._conoutSocketWorker = { dispose: () => { disposed = true; } }; + + agent.kill(); + + assert.strictEqual(disposed, true); + }); + }); + describe('argsToCommandLine', () => { describe('Plain strings', () => { it('doesn\'t quote plain string', () => { diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index d7054449516f0c9a62af351c2caa17331206d530..45a2a19fd6c161a33178cd047c9a69db31a4e313 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -34,6 +34,9 @@ export class WindowsPtyAgent { private _innerPid: number = 0; private _closeTimeout: NodeJS.Timer | undefined; private _exitCode: number | undefined; + private _processExited: boolean = false; + private _windowsAgentJob: boolean = false; + private _processExitListeners: (() => void)[] = []; private _conoutSocketWorker: ConoutConnection; private _fd: any; @@ -45,6 +48,7 @@ export class WindowsPtyAgent { public get fd(): any { return this._fd; } public get innerPid(): number { return this._innerPid; } public get pty(): number { return this._pty; } + public get windowsAgentJob(): boolean { return this._windowsAgentJob; } constructor( file: string, @@ -56,11 +60,17 @@ export class WindowsPtyAgent { debug: boolean, private _useConpty: boolean | undefined, private _useConptyDll: boolean = false, - conptyInheritCursor: boolean = false + conptyInheritCursor: boolean = false, + windowsAgentJob: boolean = false ) { if (this._useConpty === undefined || this._useConpty === true) { this._useConpty = this._getWindowsBuildNumber() >= 18309; } + // Why: winpty cannot provide spawn-time Job ownership; launching anyway can orphan agents. + if (windowsAgentJob && !this._useConpty) { + throw new Error('Windows agent Job ownership requires ConPTY'); + } + this._windowsAgentJob = windowsAgentJob; if (this._useConpty) { if (!conptyNative) { conptyNative = loadNativeModule('conpty').module; @@ -100,7 +110,7 @@ export class WindowsPtyAgent { this._outSocket.setEncoding('utf8'); // The conout socket must be ready out on another thread to avoid deadlocks this._conoutSocketWorker = new ConoutConnection(term.conout, this._useConptyDll); - this._conoutSocketWorker.onReady(() => { + const conoutReady = this._conoutSocketWorker.onReady(() => { this._conoutSocketWorker.connectSocket(this._outSocket); }); this._outSocket.on('connect', () => { @@ -116,8 +126,18 @@ export class WindowsPtyAgent { this._inSocket.setEncoding('utf8'); if (this._useConpty) { - const connect = (this._ptyNative as IConptyNative).connect(this._pty, commandLine, cwd, env, this._useConptyDll, c => this._$onProcessExit(c)); - this._innerPid = connect.pid; + try { + const connect = (this._ptyNative as IConptyNative).connect(this._pty, commandLine, cwd, env, this._useConptyDll, this._windowsAgentJob, c => this._$onProcessExit(c)); + this._innerPid = connect.pid; + } catch (error) { + // Why: native connect failures happen after JS pipe resources exist; + // stop READY from reconnecting the abandoned output socket during cleanup. + conoutReady.dispose(); + this._inSocket.destroy(); + this._outSocket.destroy(); + this._conoutSocketWorker.dispose(); + throw error; + } } } @@ -138,6 +158,24 @@ export class WindowsPtyAgent { } } + public terminateJobTree(timeoutMs: number): Promise | undefined { + if (!this._windowsAgentJob) { + return undefined; + } + if (this._processExited) { + return Promise.resolve(true); + } + return (this._ptyNative as IConptyNative).terminateJobTree?.(this._pty, timeoutMs); + } + + public onProcessExit(listener: () => void): void { + if (this._processExited) { + listener(); + return; + } + this._processExitListeners.push(listener); + } + public kill(): void { // Tell the agent to kill the pty, this releases handles to the process if (this._useConpty) { @@ -159,9 +197,14 @@ export class WindowsPtyAgent { // Close the input write handle to signal the end of session. this._inSocket.destroy(); (this._ptyNative as IConptyNative).kill(this._pty, this._useConptyDll); - this._outSocket.on('data', () => { + if (this._windowsAgentJob) { + // Why: Job termination can remove every writer before another data event. this._conoutSocketWorker.dispose(); - }); + } else { + this._outSocket.on('data', () => { + this._conoutSocketWorker.dispose(); + }); + } } } else { // Because pty.kill closes the handle, it will kill most processes by itself. @@ -223,6 +266,10 @@ export class WindowsPtyAgent { */ private _$onProcessExit(exitCode: number): void { this._exitCode = exitCode; + this._processExited = true; + const listeners = this._processExitListeners; + this._processExitListeners = []; + listeners.forEach(listener => listener()); if (!this._useConptyDll) { this._flushDataAndCleanUp(); this._outSocket.on('data', () => this._flushDataAndCleanUp()); diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index 8f1274edf6dcf6e9257ba9d711099791391ec778..8c256e56c8c344584fc5b9ef8e670691a7feba25 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -92,6 +92,18 @@ function pollForProcessTreeSize(pid: number, size: number, intervalMs: number = } if (process.platform === 'win32') { + describe('WindowsTerminal Job capability fallback', () => { + it('fails closed before a requested Job can fall back to winpty', () => { + assert.throws( + () => new WindowsTerminal('cmd.exe', '/C exit', { + useConpty: false, + windowsAgentJob: true + }), + /Windows agent Job ownership requires ConPTY/ + ); + }); + }); + [[false, false], [true, false], [true, true]].forEach(([useConpty, useConptyDll]) => { describe(`WindowsTerminal (useConpty = ${useConpty}, useConptyDll = ${useConptyDll})`, () => { describe('kill', () => { diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts index 13f6c6dbb3d7965c4699568f4704be3b6a33d947..db6ddd94a35b034c0fb998003e4332c28512809d 100644 --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -48,8 +48,23 @@ export class WindowsTerminal extends Terminal { this._deferreds = []; // Create new termal. - this._agent = new WindowsPtyAgent(file, args, parsedEnv, cwd, this._cols, this._rows, false, opt.useConpty, opt.useConptyDll, opt.conptyInheritCursor); + this._agent = new WindowsPtyAgent(file, args, parsedEnv, cwd, this._cols, this._rows, false, opt.useConpty, opt.useConptyDll, opt.conptyInheritCursor, opt.windowsAgentJob); this._socket = this._agent.outSocket; + let socketClosed = false; + let agentProcessExited = false; + let exitEmitted = false; + const emitExitWhenSafe = (): void => { + if (exitEmitted || !socketClosed || (this._agent.windowsAgentJob && !agentProcessExited)) { + return; + } + exitEmitted = true; + this.emit('exit', this._agent.exitCode); + this._close(); + }; + this._agent.onProcessExit(() => { + agentProcessExited = true; + emitExitWhenSafe(); + }); // Not available until `ready` event emitted. this._pid = this._agent.innerPid; @@ -103,8 +118,10 @@ export class WindowsTerminal extends Terminal { // Cleanup after the socket is closed. this._socket.on('close', () => { - this.emit('exit', this._agent.exitCode); - this._close(); + // Why: a Job-owned root can close ConPTY output before the native exit + // callback proves every descendant is gone; public exit needs both facts. + socketClosed = true; + emitExitWhenSafe(); }); }); @@ -161,6 +178,10 @@ export class WindowsTerminal extends Terminal { }); } + public terminateJobTree(timeoutMs: number): Promise | undefined { + return this._agent.terminateJobTree(timeoutMs); + } + public kill(signal?: string): void { this._deferNoArgs(() => { if (signal) { diff --git a/typings/node-pty.d.ts b/typings/node-pty.d.ts index 6f050ff1d97b8d3477be7ee95d2af516bd9517b7..b26831f60d2aba6cc352fad561cd0c3f7b9bfe02 100644 --- a/typings/node-pty.d.ts +++ b/typings/node-pty.d.ts @@ -108,6 +108,9 @@ declare module 'node-pty' { * @see https://docs.microsoft.com/en-us/windows/console/createpseudoconsole */ conptyInheritCursor?: boolean; + + /** Orca-specific: own this ConPTY and all descendants in a kill-on-close Job. */ + windowsAgentJob?: boolean; } /** @@ -183,6 +186,9 @@ declare module 'node-pty' { */ kill(signal?: string): void; + /** Terminates and waits for the Windows ConPTY Job Object to drain. */ + terminateJobTree?(timeoutMs: number): Promise | undefined; + /** * Pauses the pty for customizable flow control. */