diff --git a/publish.yml b/publish.yml index 800797e10..9406cc3b2 100644 --- a/publish.yml +++ b/publish.yml @@ -61,6 +61,6 @@ extends: tag: beta publishRequiresApproval: false - apiScanExcludes: 'package/third_party/conpty/1.20.240626001/win10-arm64/*.*' + apiScanExcludes: 'package/third_party/conpty/1.22.250204002/win10-arm64/*.*' apiScanSoftwareName: 'vscode-node-pty' apiScanSoftwareVersion: '1' diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 03e4e3795..1dd4cd9f0 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -34,16 +34,10 @@ typedef HRESULT (__stdcall *PFNCREATEPSEUDOCONSOLE)(COORD c, HANDLE hIn, HANDLE typedef HRESULT (__stdcall *PFNRESIZEPSEUDOCONSOLE)(HPCON hpc, COORD newSize); typedef HRESULT (__stdcall *PFNCLEARPSEUDOCONSOLE)(HPCON hpc); typedef void (__stdcall *PFNCLOSEPSEUDOCONSOLE)(HPCON hpc); +typedef void (__stdcall *PFNRELEASEPSEUDOCONSOLE)(HPCON hpc); #endif -typedef struct _PseudoConsole -{ - HANDLE hSignal; - HANDLE hPtyReference; - HANDLE hConPtyProcess; -} PseudoConsole; - struct pty_baton { int id; HANDLE hIn; @@ -51,7 +45,6 @@ struct pty_baton { HPCON hpc; HANDLE hShell; - HANDLE pty_job_handle; pty_baton(int _id, HANDLE _hIn, HANDLE _hOut, HPCON _hpc) : id(_id), hIn(_hIn), hOut(_hOut), hpc(_hpc) {}; }; @@ -80,27 +73,6 @@ static bool remove_pty_baton(int id) { return false; } -static BOOL InitPtyJobHandle(HANDLE* pty_job_handle) { - SECURITY_ATTRIBUTES attr; - memset(&attr, 0, sizeof(attr)); - attr.bInheritHandle = FALSE; - *pty_job_handle = CreateJobObjectW(&attr, nullptr); - if (*pty_job_handle == NULL) { - return FALSE; - } - JOBOBJECT_EXTENDED_LIMIT_INFORMATION limits = {}; - limits.BasicLimitInformation.LimitFlags = - JOB_OBJECT_LIMIT_DIE_ON_UNHANDLED_EXCEPTION | - JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; - if (!SetInformationJobObject(*pty_job_handle, - JobObjectExtendedLimitInformation, - &limits, - sizeof(limits))) { - return FALSE; - } - return TRUE; -} - struct ExitEvent { int exit_code = 0; }; @@ -130,11 +102,6 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) { // Get process exit code. GetExitCodeProcess(baton->hShell, (LPDWORD)(&exit_event->exit_code)); // Clean up handles - // Calling DisconnectNamedPipes here or in PtyKill results in a crash, - // ref https://github.com/microsoft/node-pty/issues/512, - // so we only call CloseHandle for now. - CloseHandle(baton->hIn); - CloseHandle(baton->hOut); CloseHandle(baton->hShell); assert(remove_pty_baton(baton->id)); @@ -459,14 +426,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { throw errorWithCode(info, "Cannot create process"); } - if (useConptyDll) { - if (!InitPtyJobHandle(&handle->pty_job_handle)) { - throw errorWithCode(info, "Initialize pty job handle failed"); - } - - PseudoConsole* server = reinterpret_cast(handle->hpc); - if (!AssignProcessToJobObject(handle->pty_job_handle, server->hConPtyProcess)) { - throw errorWithCode(info, "AssignProcessToJobObject for server failed"); + HANDLE hLibrary = LoadConptyDll(info, useConptyDll); + bool fLoadedDll = hLibrary != nullptr; + if (fLoadedDll) + { + PFNRELEASEPSEUDOCONSOLE const pfnReleasePseudoConsole = (PFNRELEASEPSEUDOCONSOLE)GetProcAddress( + (HMODULE)hLibrary, "ConptyReleasePseudoConsole"); + if (pfnReleasePseudoConsole) + { + pfnReleasePseudoConsole(handle->hpc); } } @@ -475,6 +443,9 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { // Close the thread handle to avoid resource leak CloseHandle(piClient.hThread); + // Close the input read and output write handle of the pseudoconsole + CloseHandle(handle->hIn); + CloseHandle(handle->hOut); SetupExitCallback(env, exitCallback, handle); @@ -588,11 +559,7 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) { pfnClosePseudoConsole(handle->hpc); } } - TerminateProcess(handle->hShell, 1); - if (useConptyDll) { - CloseHandle(handle->pty_job_handle); - } } return env.Undefined(); diff --git a/src/windowsConoutConnection.ts b/src/windowsConoutConnection.ts index de0b6b498..56243bb0a 100644 --- a/src/windowsConoutConnection.ts +++ b/src/windowsConoutConnection.ts @@ -54,10 +54,6 @@ export class ConoutConnection implements IDisposable { } dispose(): void { - if (this._isDisposed) { - return; - } - this._isDisposed = true; // Drain all data from the socket before closing this._drainDataAndClose(); } diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 59e14096b..78d08b9fa 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -158,11 +158,11 @@ export class WindowsPtyAgent { } public kill(): void { - this._inSocket.readable = false; - this._outSocket.readable = false; // Tell the agent to kill the pty, this releases handles to the process if (this._useConpty) { if (!this._useConptyDll) { + this._inSocket.readable = false; + this._outSocket.readable = false; this._getConsoleProcessList().then(consoleProcessList => { consoleProcessList.forEach((pid: number) => { try { @@ -172,8 +172,16 @@ export class WindowsPtyAgent { } }); }); + (this._ptyNative as IConptyNative).kill(this._pty, this._useConptyDll); + this._conoutSocketWorker.dispose(); + } else { + // 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', () => { + this._conoutSocketWorker.dispose(); + }); } - (this._ptyNative as IConptyNative).kill(this._pty, this._useConptyDll); } else { // Because pty.kill closes the handle, it will kill most processes by itself. // Process IDs can be reused as soon as all handles to them are @@ -191,7 +199,6 @@ export class WindowsPtyAgent { } }); } - this._conoutSocketWorker.dispose(); } private _getConsoleProcessList(): Promise { @@ -235,11 +242,16 @@ export class WindowsPtyAgent { */ private _$onProcessExit(exitCode: number): void { this._exitCode = exitCode; - this._flushDataAndCleanUp(); - this._outSocket.on('data', () => this._flushDataAndCleanUp()); + if (!this._useConptyDll) { + this._flushDataAndCleanUp(); + this._outSocket.on('data', () => this._flushDataAndCleanUp()); + } } private _flushDataAndCleanUp(): void { + if (this._useConptyDll) { + return; + } if (this._closeTimeout) { clearTimeout(this._closeTimeout); } @@ -247,6 +259,9 @@ export class WindowsPtyAgent { } private _cleanUpProcess(): void { + if (this._useConptyDll) { + return; + } this._inSocket.readable = false; this._outSocket.readable = false; this._outSocket.destroy(); diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index b2b91df3f..705ae5045 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -26,13 +26,16 @@ function pollForProcessState(desiredState: IProcessState, intervalMs: number = 1 psList({ all: true }).then(ps => { let success = true; const pids = Object.keys(desiredState).map(k => parseInt(k, 10)); + console.log('expected pids', JSON.stringify(pids)); pids.forEach(pid => { if (desiredState[pid]) { if (!ps.some(p => p.pid === pid)) { + console.log(`pid ${pid} does not exist`); success = false; } } else { if (ps.some(p => p.pid === pid)) { + console.log(`pid ${pid} still exists`); success = false; } } @@ -71,6 +74,7 @@ function pollForProcessTreeSize(pid: number, size: number, intervalMs: number = }).forEach(p => openList.push(p)); list.push(current); } + console.log('list', JSON.stringify(list)); const success = list.length === size; if (success) { clearInterval(interval); @@ -88,37 +92,33 @@ function pollForProcessTreeSize(pid: number, size: number, intervalMs: number = } if (process.platform === 'win32') { - [[true, false], [true, true], [false, false]].forEach(([useConpty, useConptyDll]) => { + [[false, false], [true, false], [true, true]].forEach(([useConpty, useConptyDll]) => { describe(`WindowsTerminal (useConpty = ${useConpty}, useConptyDll = ${useConptyDll})`, () => { describe('kill', () => { - it('should not crash parent process', (done) => { + it('should not crash parent process', function (done) { + this.timeout(20000); const term = new WindowsTerminal('cmd.exe', [], { useConpty, useConptyDll }); + term.on('exit', () => done()); term.kill(); - // Add done call to deferred function queue to ensure the kill call has completed - (term)._defer(done); }); it('should kill the process tree', function (done: Mocha.Done): void { - this.timeout(10000); + this.timeout(20000); const term = new WindowsTerminal('cmd.exe', [], { useConpty, useConptyDll }); // Start sub-processes term.write('powershell.exe\r'); - term.write('notepad.exe\r'); term.write('node.exe\r'); - pollForProcessTreeSize(term.pid, 4, 500, 5000).then(list => { + 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(), 'notepad.exe'); - assert.strictEqual(list[3].name.toLowerCase(), 'node.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] = true; - desiredState[list[3].pid] = false; + desiredState[list[2].pid] = false; term.on('exit', () => { - pollForProcessState(desiredState).then(() => { - // Kill notepad before done - process.kill(list[2].pid); + pollForProcessState(desiredState, 1000, 5000).then(() => { done(); }); }); @@ -127,11 +127,15 @@ if (process.platform === 'win32') { }); describe('resize', () => { - it('should throw a non-native exception when resizing an invalid value', () => { + it('should throw a non-native exception when resizing an invalid value', (done) => { const term = new WindowsTerminal('cmd.exe', [], { useConpty, useConptyDll }); assert.throws(() => term.resize(-1, -1)); assert.throws(() => term.resize(0, 0)); assert.doesNotThrow(() => term.resize(1, 1)); + term.on('exit', () => { + done(); + }); + term.kill(); }); it('should throw a non-native exception when resizing a killed terminal', (done) => { const term = new WindowsTerminal('cmd.exe', [], { useConpty, useConptyDll }); @@ -146,7 +150,8 @@ if (process.platform === 'win32') { }); describe('Args as CommandLine', () => { - it('should not fail running a file containing a space in the path', (done) => { + it('should not fail running a file containing a space in the path', function (done) { + this.timeout(10000); const spaceFolder = path.resolve(__dirname, '..', 'fixtures', 'space folder'); if (!fs.existsSync(spaceFolder)) { fs.mkdirSync(spaceFolder); @@ -173,8 +178,9 @@ if (process.platform === 'win32') { }); describe('env', () => { - it('should set environment variables of the shell', (done) => { - const term = new WindowsTerminal('cmd.exe', '/C echo %FOO%', { env: { FOO: 'BAR' }}); + it('should set environment variables of the shell', function (done) { + this.timeout(10000); + const term = new WindowsTerminal('cmd.exe', '/C echo %FOO%', { useConpty, useConptyDll, env: { FOO: 'BAR' }}); let result = ''; term.on('data', (data) => { result += data; @@ -187,7 +193,8 @@ if (process.platform === 'win32') { }); describe('On close', () => { - it('should return process zero exit codes', (done) => { + it('should return process zero exit codes', function (done) { + this.timeout(10000); const term = new WindowsTerminal('cmd.exe', '/C exit', { useConpty, useConptyDll }); term.on('exit', (code) => { assert.strictEqual(code, 0); @@ -195,7 +202,8 @@ if (process.platform === 'win32') { }); }); - it('should return process non-zero exit codes', (done) => { + it('should return process non-zero exit codes', function (done) { + this.timeout(10000); const term = new WindowsTerminal('cmd.exe', '/C exit 2', { useConpty, useConptyDll }); term.on('exit', (code) => { assert.strictEqual(code, 2); @@ -205,7 +213,8 @@ if (process.platform === 'win32') { }); describe('Write', () => { - it('should accept input', (done) => { + it('should accept input', function (done) { + this.timeout(10000); const term = new WindowsTerminal('cmd.exe', '', { useConpty, useConptyDll }); term.write('exit\r'); term.on('exit', () => { diff --git a/src/worker/conoutSocketWorker.ts b/src/worker/conoutSocketWorker.ts index cedd86fe2..296f91a92 100644 --- a/src/worker/conoutSocketWorker.ts +++ b/src/worker/conoutSocketWorker.ts @@ -4,19 +4,22 @@ import { parentPort, workerData } from 'worker_threads'; import { Socket, createServer } from 'net'; +import * as fs from 'fs'; import { ConoutWorkerMessage, IWorkerData, getWorkerPipeName } from '../shared/conout'; const conoutPipeName = (workerData as IWorkerData).conoutPipeName; -const conoutSocket = new Socket(); +const outSocketFD = fs.openSync(conoutPipeName, 'r'); +const conoutSocket = new Socket({ + fd: outSocketFD, + readable: true +}); conoutSocket.setEncoding('utf8'); -conoutSocket.connect(conoutPipeName, () => { - const server = createServer(workerSocket => { - conoutSocket.pipe(workerSocket); - }); - server.listen(getWorkerPipeName(conoutPipeName)); - - if (!parentPort) { - throw new Error('worker_threads parentPort is null'); - } - parentPort.postMessage(ConoutWorkerMessage.READY); +const server = createServer(workerSocket => { + conoutSocket.pipe(workerSocket); }); +server.listen(getWorkerPipeName(conoutPipeName)); + +if (!parentPort) { + throw new Error('worker_threads parentPort is null'); +} +parentPort.postMessage(ConoutWorkerMessage.READY); diff --git a/third_party/conpty/1.20.240626001/win10-arm64/OpenConsole.exe b/third_party/conpty/1.20.240626001/win10-arm64/OpenConsole.exe deleted file mode 100644 index 664c9dd52..000000000 Binary files a/third_party/conpty/1.20.240626001/win10-arm64/OpenConsole.exe and /dev/null differ diff --git a/third_party/conpty/1.20.240626001/win10-arm64/conpty.dll b/third_party/conpty/1.20.240626001/win10-arm64/conpty.dll deleted file mode 100644 index 38b5f849c..000000000 Binary files a/third_party/conpty/1.20.240626001/win10-arm64/conpty.dll and /dev/null differ diff --git a/third_party/conpty/1.20.240626001/win10-x64/OpenConsole.exe b/third_party/conpty/1.20.240626001/win10-x64/OpenConsole.exe deleted file mode 100644 index 1fae768ab..000000000 Binary files a/third_party/conpty/1.20.240626001/win10-x64/OpenConsole.exe and /dev/null differ diff --git a/third_party/conpty/1.20.240626001/win10-x64/conpty.dll b/third_party/conpty/1.20.240626001/win10-x64/conpty.dll deleted file mode 100644 index dc76803cd..000000000 Binary files a/third_party/conpty/1.20.240626001/win10-x64/conpty.dll and /dev/null differ diff --git a/third_party/conpty/1.22.250204002/win10-arm64/OpenConsole.exe b/third_party/conpty/1.22.250204002/win10-arm64/OpenConsole.exe new file mode 100644 index 000000000..eca3af317 Binary files /dev/null and b/third_party/conpty/1.22.250204002/win10-arm64/OpenConsole.exe differ diff --git a/third_party/conpty/1.22.250204002/win10-arm64/conpty.dll b/third_party/conpty/1.22.250204002/win10-arm64/conpty.dll new file mode 100644 index 000000000..0fe88eaf3 Binary files /dev/null and b/third_party/conpty/1.22.250204002/win10-arm64/conpty.dll differ diff --git a/third_party/conpty/1.22.250204002/win10-x64/OpenConsole.exe b/third_party/conpty/1.22.250204002/win10-x64/OpenConsole.exe new file mode 100644 index 000000000..2b637b8cc Binary files /dev/null and b/third_party/conpty/1.22.250204002/win10-x64/OpenConsole.exe differ diff --git a/third_party/conpty/1.22.250204002/win10-x64/conpty.dll b/third_party/conpty/1.22.250204002/win10-x64/conpty.dll new file mode 100644 index 000000000..03e9414c6 Binary files /dev/null and b/third_party/conpty/1.22.250204002/win10-x64/conpty.dll differ