From a57b2f6462e785eb963648a48bd1ae74ca80b9f5 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 14 Jan 2019 11:28:05 -0800 Subject: [PATCH 01/10] Enable ConPTY by defaults from build 18309 --- src/windowsPtyAgent.ts | 2 +- typings/node-pty.d.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index bc748823a..c5b9b5b5c 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -54,7 +54,7 @@ export class WindowsPtyAgent { private _useConpty: boolean | undefined ) { if (this._useConpty === undefined || this._useConpty === true) { - this._useConpty = this._getWindowsBuildNumber() >= 17692; + this._useConpty = this._getWindowsBuildNumber() >= 18309; } if (this._useConpty) { if (!conptyNative) { diff --git a/typings/node-pty.d.ts b/typings/node-pty.d.ts index 30aa14b70..7c2196ca2 100644 --- a/typings/node-pty.d.ts +++ b/typings/node-pty.d.ts @@ -28,7 +28,8 @@ declare module 'node-pty' { encoding?: string; /** * Whether to use the experimental ConPTY system on Windows. When this is not set, ConPTY will - * be used when the Windows build number is >= 17692. + * be used when the Windows build number is >= 18309 (it's available in 17692 but is considered + * unstable). * * This setting does nothing on non-Windows. */ From 6c5e3160bd4875243d35285c6407e535bcd04f2c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 14 Jan 2019 12:59:02 -0800 Subject: [PATCH 02/10] WIP --- examples/killDeepTree/index.js | 2 +- src/native.d.ts | 1 + src/win/conpty.cc | 44 ++++++++++++++++++++++++++++++++++ src/windowsPtyAgent.ts | 28 ++++++++++++---------- src/windowsTerminal.test.ts | 11 +++++++++ typings/node-pty.d.ts | 4 ++-- 6 files changed, 74 insertions(+), 16 deletions(-) diff --git a/examples/killDeepTree/index.js b/examples/killDeepTree/index.js index 8fc98dd65..9a0019278 100644 --- a/examples/killDeepTree/index.js +++ b/examples/killDeepTree/index.js @@ -12,7 +12,7 @@ var ptyProcess = pty.spawn(shell, [], { }); ptyProcess.on('data', function(data) { - console.log(data); + process.stdout.write(data); }); ptyProcess.write('start notepad\r'); diff --git a/src/native.d.ts b/src/native.d.ts index 22cde78c2..a21b73217 100644 --- a/src/native.d.ts +++ b/src/native.d.ts @@ -5,6 +5,7 @@ interface IConptyNative { startProcess(file: string, cols: number, rows: number, debug: boolean, pipeName: string): IConptyProcess; connect(ptyId: number, commandLine: string, cwd: string, env: string[], onExitCallback: (exitCode: number) => void): { pid: number }; + getProcessList(shellPid: number): number[]; resize(ptyId: number, cols: number, rows: number): void; kill(ptyId: number): void; } diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 2702bec6b..63759d8a3 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -419,9 +419,52 @@ static NAN_METHOD(PtyKill) { } } + // Fetch the console process tree + // auto processList = std::vector(64); + // auto processCount = GetConsoleProcessList(&processList[0], processList.size()); + // if (processList.size() < processCount) { + // processList.resize(processCount); + // processCount = GetConsoleProcessList(&processList[0], processList.size()); + // } + + // // Kill the console process tree + // for (DWORD i = 0; i < processCount; i++) { + // TerminateProcess(processList[i], 1); + // } + + // Close the shell handle + CloseHandle(handle->hShell); + return info.GetReturnValue().SetUndefined(); } +static NAN_METHOD(PtyGetProcessList) { + Nan::HandleScope scope; + + if (info.Length() != 1 || + !info[0]->IsNumber()) { + Nan::ThrowError("Usage: pty.getProcessList(shellPid)"); + return; + } + + int shellPid = info[0]->Int32Value(); + + AttachConsole(shellPid); + auto processList = std::vector(64); + auto processCount = GetConsoleProcessList(&processList[0], processList.size()); + if (processList.size() < processCount) { + processList.resize(processCount); + processCount = GetConsoleProcessList(&processList[0], processList.size()); + } + FreeConsole(); + + auto result = Nan::New(processCount); + for (DWORD i = 0; i < processCount; i++) { + result->Set(i, Nan::New(processList[i])); + } + info.GetReturnValue().Set(result); +} + /** * Init */ @@ -432,6 +475,7 @@ extern "C" void init(v8::Handle target) { Nan::SetMethod(target, "connect", PtyConnect); Nan::SetMethod(target, "resize", PtyResize); Nan::SetMethod(target, "kill", PtyKill); + Nan::SetMethod(target, "getProcessList", PtyGetProcessList); }; NODE_MODULE(pty, init); diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index c5b9b5b5c..925984ea2 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -132,21 +132,23 @@ export class WindowsPtyAgent { if (this._useConpty) { (this._ptyNative as IConptyNative).kill(this._pty); } else { - const processList: number[] = (this._ptyNative as IWinptyNative).getProcessList(this._pid); (this._ptyNative as IWinptyNative).kill(this._pid, this._innerPidHandle); - // Since pty.kill will kill most processes by itself and process IDs can be - // reused as soon as all handles to them are dropped, we want to immediately - // kill the entire console process list. If we do not force kill all - // processes here, node servers in particular seem to become detached and - // remain running (see Microsoft/vscode#26807). - processList.forEach(pid => { - try { - process.kill(pid); - } catch (e) { - // Ignore if process cannot be found (kill ESRCH error) - } - }); } + // Since pty.kill closes the handle it will kill most processes by itself + // and process IDs can be reused as soon as all handles to them are + // dropped, we want to immediately kill the entire console process list. + // If we do not force kill all processes here, node servers in particular + // seem to become detached and remain running (see + // Microsoft/vscode#26807). + const processList: number[] = this._ptyNative.getProcessList(this._useConpty ? this._innerPid : this._pid); + console.log('processList', processList.join(', ')); + processList.forEach(pid => { + try { + process.kill(pid); + } catch (e) { + // Ignore if process cannot be found (kill ESRCH error) + } + }); } public get exitCode(): number { diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index 49f1da918..dc58857d5 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -3,6 +3,7 @@ * Copyright (c) 2018, Microsoft Corporation (MIT License). */ +import * as cp from 'child_process'; import * as fs from 'fs'; import * as assert from 'assert'; import { WindowsTerminal } from './windowsTerminal'; @@ -17,6 +18,16 @@ if (process.platform === 'win32') { // Add done call to deferred function queue to ensure the kill call has completed (term)._defer(done); }); + it('should kill the process tree', (done) => { + const term = new WindowsTerminal('cmd.exe', [], {}); + // Start a sub-process + term.write('powershell.exe'); + const proc = cp.execSync(`tasklist /fi "PID eq ${term.pid}"`); + const index = proc.toString().indexOf(term.pid.toString()); + console.log('******* ' + index + ', ' + proc.toString()); + // term.pid + // term.kill(); + }); }); describe('resize', () => { diff --git a/typings/node-pty.d.ts b/typings/node-pty.d.ts index 7c2196ca2..d3932fb6b 100644 --- a/typings/node-pty.d.ts +++ b/typings/node-pty.d.ts @@ -28,8 +28,8 @@ declare module 'node-pty' { encoding?: string; /** * Whether to use the experimental ConPTY system on Windows. When this is not set, ConPTY will - * be used when the Windows build number is >= 18309 (it's available in 17692 but is considered - * unstable). + * be used when the Windows build number is >= 18309 (it's available in 17134 and 17692 but is + * too unstable to enable by default). * * This setting does nothing on non-Windows. */ From d3eaef3c7da0355b46743598de80dafceaf61611 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 18 Jan 2019 11:30:31 -0800 Subject: [PATCH 03/10] Fix kill under conpty --- binding.gyp | 9 ++++ package.json | 6 ++- scripts/post-install.js | 2 + src/conpty_console_list_agent.ts | 15 +++++++ src/native.d.ts | 1 - src/win/conpty.cc | 42 ------------------ src/win/conpty_console_list.cc | 43 ++++++++++++++++++ src/windowsPtyAgent.ts | 55 ++++++++++++++++------- src/windowsTerminal.test.ts | 75 +++++++++++++++++++++++++++++--- tsconfig.json | 2 +- 10 files changed, 183 insertions(+), 67 deletions(-) create mode 100644 src/conpty_console_list_agent.ts create mode 100644 src/win/conpty_console_list.cc diff --git a/binding.gyp b/binding.gyp index 7fa4d0023..0c6e3b086 100644 --- a/binding.gyp +++ b/binding.gyp @@ -15,6 +15,15 @@ 'shlwapi.lib' ] }, + { + 'target_name': 'conpty_console_list', + 'include_dirs' : [ + ' void): { pid: number }; - getProcessList(shellPid: number): number[]; resize(ptyId: number, cols: number, rows: number): void; kill(ptyId: number): void; } diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 63759d8a3..1b5a74949 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -419,52 +419,11 @@ static NAN_METHOD(PtyKill) { } } - // Fetch the console process tree - // auto processList = std::vector(64); - // auto processCount = GetConsoleProcessList(&processList[0], processList.size()); - // if (processList.size() < processCount) { - // processList.resize(processCount); - // processCount = GetConsoleProcessList(&processList[0], processList.size()); - // } - - // // Kill the console process tree - // for (DWORD i = 0; i < processCount; i++) { - // TerminateProcess(processList[i], 1); - // } - - // Close the shell handle CloseHandle(handle->hShell); return info.GetReturnValue().SetUndefined(); } -static NAN_METHOD(PtyGetProcessList) { - Nan::HandleScope scope; - - if (info.Length() != 1 || - !info[0]->IsNumber()) { - Nan::ThrowError("Usage: pty.getProcessList(shellPid)"); - return; - } - - int shellPid = info[0]->Int32Value(); - - AttachConsole(shellPid); - auto processList = std::vector(64); - auto processCount = GetConsoleProcessList(&processList[0], processList.size()); - if (processList.size() < processCount) { - processList.resize(processCount); - processCount = GetConsoleProcessList(&processList[0], processList.size()); - } - FreeConsole(); - - auto result = Nan::New(processCount); - for (DWORD i = 0; i < processCount; i++) { - result->Set(i, Nan::New(processList[i])); - } - info.GetReturnValue().Set(result); -} - /** * Init */ @@ -475,7 +434,6 @@ extern "C" void init(v8::Handle target) { Nan::SetMethod(target, "connect", PtyConnect); Nan::SetMethod(target, "resize", PtyResize); Nan::SetMethod(target, "kill", PtyKill); - Nan::SetMethod(target, "getProcessList", PtyGetProcessList); }; NODE_MODULE(pty, init); diff --git a/src/win/conpty_console_list.cc b/src/win/conpty_console_list.cc new file mode 100644 index 000000000..72102360e --- /dev/null +++ b/src/win/conpty_console_list.cc @@ -0,0 +1,43 @@ +/** + * Copyright (c) 2019, Microsoft Corporation (MIT License). + */ + +#include +#include + +static NAN_METHOD(ApiConsoleProcessList) { + if (info.Length() != 1 || + !info[0]->IsNumber()) { + Nan::ThrowError("Usage: getConsoleProcessList(shellPid)"); + return; + } + + const SHORT pid = info[0]->Uint32Value(); + + if (!FreeConsole()) { + Nan::ThrowError("FreeConsole failed"); + } + if (!AttachConsole(pid)) { + Nan::ThrowError("AttachConsole failed"); + } + auto processList = std::vector(64); + auto processCount = GetConsoleProcessList(&processList[0], processList.size()); + if (processList.size() < processCount) { + processList.resize(processCount); + processCount = GetConsoleProcessList(&processList[0], processList.size()); + } + FreeConsole(); + + v8::Local result = Nan::New(); + for (DWORD i = 0; i < processCount; i++) { + result->Set(i, Nan::New(processList[i])); + } + info.GetReturnValue().Set(result); +} + +extern "C" void init(v8::Handle target) { + Nan::HandleScope scope; + Nan::SetMethod(target, "getConsoleProcessList", ApiConsoleProcessList); +}; + +NODE_MODULE(pty, init); diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 925984ea2..8c14229f3 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -9,6 +9,7 @@ import * as path from 'path'; import { Socket } from 'net'; import { ArgvOrCommandLine } from './types'; import { loadNative } from './utils'; +import { fork } from 'child_process'; let conptyNative: IConptyNative; let winptyNative: IWinptyNative; @@ -130,24 +131,48 @@ export class WindowsPtyAgent { this._outSocket.writable = false; // Tell the agent to kill the pty, this releases handles to the process if (this._useConpty) { - (this._ptyNative as IConptyNative).kill(this._pty); + this._getConsoleProcessList().then(consoleProcessList => { + consoleProcessList.forEach((pid: number) => { + try { + process.kill(pid); + } catch (e) { + // Ignore if process cannot be found (kill ESRCH error) + } + }); + (this._ptyNative as IConptyNative).kill(this._pty); + }); } else { (this._ptyNative as IWinptyNative).kill(this._pid, this._innerPidHandle); + // Since pty.kill closes the handle it will kill most processes by itself + // and process IDs can be reused as soon as all handles to them are + // dropped, we want to immediately kill the entire console process list. + // If we do not force kill all processes here, node servers in particular + // seem to become detached and remain running (see + // Microsoft/vscode#26807). + const processList: number[] = (this._ptyNative as IWinptyNative).getProcessList(this._pid); + processList.forEach(pid => { + try { + process.kill(pid); + } catch (e) { + // Ignore if process cannot be found (kill ESRCH error) + } + }); } - // Since pty.kill closes the handle it will kill most processes by itself - // and process IDs can be reused as soon as all handles to them are - // dropped, we want to immediately kill the entire console process list. - // If we do not force kill all processes here, node servers in particular - // seem to become detached and remain running (see - // Microsoft/vscode#26807). - const processList: number[] = this._ptyNative.getProcessList(this._useConpty ? this._innerPid : this._pid); - console.log('processList', processList.join(', ')); - processList.forEach(pid => { - try { - process.kill(pid); - } catch (e) { - // Ignore if process cannot be found (kill ESRCH error) - } + } + + private _getConsoleProcessList(): Promise { + return new Promise(resolve => { + const agent = fork(path.join(__dirname, 'conpty_console_list_agent'), [ this._innerPid.toString() ]); + agent.on('message', message => { + clearTimeout(timeout); + resolve(message.consoleProcessList); + }); + const timeout = setTimeout(() => { + // Something went wrong, just send back the shell PID + console.error('Could not fetch console process list'); + agent.kill(); + resolve([ this._innerPid ]); + }, 5000); }); } diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index dc58857d5..1b1b499b4 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -8,6 +8,48 @@ import * as fs from 'fs'; import * as assert from 'assert'; import { WindowsTerminal } from './windowsTerminal'; import * as path from 'path'; +import { getProcessList } from 'windows-process-tree'; +import * as psList from 'ps-list'; + +interface IProcessState { + // Whether the PID must exist or must not exist + [pid: number]: boolean; +} + +function pollForProcessState(desiredState: IProcessState, intervalMs: number = 100, timeoutMs: number = 2000): Promise { + return new Promise(resolve => { + let tries = 0; + const interval = setInterval(() => { + psList({ all: true }).then(ps => { + let success = true; + const pids = Object.keys(desiredState).map(k => parseInt(k, 10)); + pids.forEach(pid => { + if (desiredState[pid]) { + if (!ps.some(p => p.pid === pid)) { + success = false; + } + } else { + if (ps.some(p => p.pid === pid)) { + success = false; + } + } + }); + if (success) { + clearInterval(interval); + resolve(); + return; + } + tries++; + if (tries * intervalMs >= timeoutMs) { + clearInterval(interval); + const processListing = pids.map(k => `${k}: ${desiredState[k]}`).join('\n'); + assert.fail(`Bad process state, expected:\n${processListing}`); + resolve(); + } + }); + }, intervalMs); + }); +} if (process.platform === 'win32') { describe('WindowsTerminal', () => { @@ -18,15 +60,34 @@ if (process.platform === 'win32') { // Add done call to deferred function queue to ensure the kill call has completed (term)._defer(done); }); - it('should kill the process tree', (done) => { + it('should kill the process tree', function (done: Mocha.Done): void { + this.timeout(5000); + const term = new WindowsTerminal('cmd.exe', [], {}); // Start a sub-process - term.write('powershell.exe'); - const proc = cp.execSync(`tasklist /fi "PID eq ${term.pid}"`); - const index = proc.toString().indexOf(term.pid.toString()); - console.log('******* ' + index + ', ' + proc.toString()); - // term.pid - // term.kill(); + term.write('powershell.exe\r'); + term.write('notepad.exe\r'); + term.write('node.exe\r'); + setTimeout(() => { + getProcessList(term.pid, list => { + assert.equal(list.length, 4); + assert.equal(list[0].name, 'cmd.exe'); + assert.equal(list[1].name, 'powershell.exe'); + assert.equal(list[2].name, 'notepad.exe'); + assert.equal(list[3].name, '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; + pollForProcessState(desiredState).then(() => { + // Kill notepad before done + process.kill(list[2].pid); + done(); + }); + }); + }, 1000); }); }); diff --git a/tsconfig.json b/tsconfig.json index 172c415e2..adac5354f 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -6,7 +6,7 @@ "outDir": "lib", "sourceMap": true, "lib": [ - "es5" + "es2015" ], "alwaysStrict": true, "noImplicitAny": true, From 93e8af914ce6e1b9968ba6a6659e5f86750ea523 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 18 Jan 2019 11:37:41 -0800 Subject: [PATCH 04/10] Make windows-process-tree an optional dep --- package.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 01b645335..de9e6d638 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,9 @@ "ps-list": "^6.0.0", "tslint": "^5.12.1", "tslint-consistent-codestyle": "^1.15.0", - "typescript": "^3.2.2", + "typescript": "^3.2.2" + }, + "optionalDependencies": { "windows-process-tree": "^0.2.3" } } From 2c7f64c2511082ca3710e59b0cfa5fc1ab68dafe Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 18 Jan 2019 11:45:17 -0800 Subject: [PATCH 05/10] Move windows-process-tree to install.js Optional dev deps aren't supported by npm --- package.json | 3 --- scripts/install.js | 26 +++++++++++++++++++------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index de9e6d638..50552e57b 100644 --- a/package.json +++ b/package.json @@ -55,8 +55,5 @@ "tslint": "^5.12.1", "tslint-consistent-codestyle": "^1.15.0", "typescript": "^3.2.2" - }, - "optionalDependencies": { - "windows-process-tree": "^0.2.3" } } diff --git a/scripts/install.js b/scripts/install.js index 4477e48b6..be0654bb9 100644 --- a/scripts/install.js +++ b/scripts/install.js @@ -4,15 +4,27 @@ const os = require('os'); const path = require('path'); const spawn = require('child_process').spawn; -const args = ['rebuild']; -if (process.env.NODE_PTY_DEBUG) { - args.push('--debug'); -} -const p = spawn(os.platform() === 'win32' ? 'node-gyp.cmd' : 'node-gyp', args, { +// windows-process-tree is an optional dev dependency which npm does not support +const npmArgs = ['i', '--no-save', 'windows-process-tree@0.2.3']; +const npmProcess = spawn(os.platform() === 'win32' ? 'npm.cmd' : 'npm', npmArgs, { cwd: path.join(__dirname, '..'), stdio: 'inherit' }); -p.on('exit', function (code) { - process.exit(code); +npmProcess.on('exit', function (code) { + if (code) { + throw new Error('npm i windows-process-tree failed with code ' + code); + } + const gypArgs = ['rebuild']; + if (process.env.NODE_PTY_DEBUG) { + gypArgs.push('--debug'); + } + const gypProcess = spawn(os.platform() === 'win32' ? 'node-gyp.cmd' : 'node-gyp', gypArgs, { + cwd: path.join(__dirname, '..'), + stdio: 'inherit' + }); + + gypProcess.on('exit', function (code) { + process.exit(code); + }); }); From 9a83950b9a6194a61659c1d80c62f9e5ea0f3f01 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 18 Jan 2019 11:49:51 -0800 Subject: [PATCH 06/10] Only install it on Windows --- scripts/install.js | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/scripts/install.js b/scripts/install.js index be0654bb9..fe91375ff 100644 --- a/scripts/install.js +++ b/scripts/install.js @@ -4,14 +4,7 @@ const os = require('os'); const path = require('path'); const spawn = require('child_process').spawn; -// windows-process-tree is an optional dev dependency which npm does not support -const npmArgs = ['i', '--no-save', 'windows-process-tree@0.2.3']; -const npmProcess = spawn(os.platform() === 'win32' ? 'npm.cmd' : 'npm', npmArgs, { - cwd: path.join(__dirname, '..'), - stdio: 'inherit' -}); - -npmProcess.on('exit', function (code) { +installWindowsProcessTree().then(() => { if (code) { throw new Error('npm i windows-process-tree failed with code ' + code); } @@ -28,3 +21,24 @@ npmProcess.on('exit', function (code) { process.exit(code); }); }); + +// windows-process-tree is an optional dev dependency which npm does not support +function installWindowsProcessTree() { + return new Promise(resolve => { + if (process.platform !== 'win32') { + resolve(); + return; + } + const npmArgs = ['i', '--no-save', 'windows-process-tree@0.2.3']; + const npmProcess = spawn(os.platform() === 'win32' ? 'npm.cmd' : 'npm', npmArgs, { + cwd: path.join(__dirname, '..'), + stdio: 'inherit' + }); + npmProcess.on('exit', function (code) { + if (code) { + throw new Error('windows-process-tree install failed with code ' + code); + } + resolve(); + }); + }); +} From 4475dcd471e1452167a3c2c1d17ec017f079c4a6 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 18 Jan 2019 11:54:05 -0800 Subject: [PATCH 07/10] Protect import of windows-process-tree --- src/windowsTerminal.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index 1b1b499b4..1d155ee83 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -8,9 +8,13 @@ import * as fs from 'fs'; import * as assert from 'assert'; import { WindowsTerminal } from './windowsTerminal'; import * as path from 'path'; -import { getProcessList } from 'windows-process-tree'; import * as psList from 'ps-list'; +let getProcessList: any; +if (process.platform === 'win32') { + getProcessList = require('windows-process-tree'); +} + interface IProcessState { // Whether the PID must exist or must not exist [pid: number]: boolean; @@ -69,7 +73,7 @@ if (process.platform === 'win32') { term.write('notepad.exe\r'); term.write('node.exe\r'); setTimeout(() => { - getProcessList(term.pid, list => { + getProcessList(term.pid, (list: {name: string, pid: number}[]) => { assert.equal(list.length, 4); assert.equal(list[0].name, 'cmd.exe'); assert.equal(list[1].name, 'powershell.exe'); From f18689276871009b6b6ec34eec030f43218dade2 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 18 Jan 2019 11:55:43 -0800 Subject: [PATCH 08/10] Fix resolve --- scripts/install.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/scripts/install.js b/scripts/install.js index fe91375ff..e36da61e6 100644 --- a/scripts/install.js +++ b/scripts/install.js @@ -5,9 +5,6 @@ const path = require('path'); const spawn = require('child_process').spawn; installWindowsProcessTree().then(() => { - if (code) { - throw new Error('npm i windows-process-tree failed with code ' + code); - } const gypArgs = ['rebuild']; if (process.env.NODE_PTY_DEBUG) { gypArgs.push('--debug'); @@ -35,9 +32,6 @@ function installWindowsProcessTree() { stdio: 'inherit' }); npmProcess.on('exit', function (code) { - if (code) { - throw new Error('windows-process-tree install failed with code ' + code); - } resolve(); }); }); From a486374ed3c629c06552dabbc93cd7204cbf85e7 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 18 Jan 2019 12:00:11 -0800 Subject: [PATCH 09/10] Fix call to getProcessList --- src/windowsTerminal.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index 1d155ee83..0b3a05acc 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -12,7 +12,7 @@ import * as psList from 'ps-list'; let getProcessList: any; if (process.platform === 'win32') { - getProcessList = require('windows-process-tree'); + getProcessList = require('windows-process-tree').getProcessList; } interface IProcessState { From f7a771b6dd86cee3319033227bf87c901099fdb0 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 18 Jan 2019 13:36:06 -0800 Subject: [PATCH 10/10] Poll for process tree --- src/windowsTerminal.test.ts | 65 +++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index 0b3a05acc..c4370cb1e 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -20,6 +20,11 @@ interface IProcessState { [pid: number]: boolean; } +interface IWindowsProcessTreeResult { + name: string; + pid: number; +} + function pollForProcessState(desiredState: IProcessState, intervalMs: number = 100, timeoutMs: number = 2000): Promise { return new Promise(resolve => { let tries = 0; @@ -55,6 +60,28 @@ function pollForProcessState(desiredState: IProcessState, intervalMs: number = 1 }); } +function pollForProcessTreeSize(pid: number, size: number, intervalMs: number = 100, timeoutMs: number = 2000): Promise { + return new Promise(resolve => { + let tries = 0; + const interval = setInterval(() => { + getProcessList(pid, (list: {name: string, pid: number}[]) => { + const success = list.length === size; + if (success) { + clearInterval(interval); + resolve(list); + return; + } + tries++; + if (tries * intervalMs >= timeoutMs) { + clearInterval(interval); + assert.fail(`Bad process state, expected: ${size}, actual: ${list.length}`); + resolve(); + } + }); + }, intervalMs); + }); +} + if (process.platform === 'win32') { describe('WindowsTerminal', () => { describe('kill', () => { @@ -66,32 +93,28 @@ if (process.platform === 'win32') { }); it('should kill the process tree', function (done: Mocha.Done): void { this.timeout(5000); - const term = new WindowsTerminal('cmd.exe', [], {}); - // Start a sub-process + // Start sub-processes term.write('powershell.exe\r'); term.write('notepad.exe\r'); term.write('node.exe\r'); - setTimeout(() => { - getProcessList(term.pid, (list: {name: string, pid: number}[]) => { - assert.equal(list.length, 4); - assert.equal(list[0].name, 'cmd.exe'); - assert.equal(list[1].name, 'powershell.exe'); - assert.equal(list[2].name, 'notepad.exe'); - assert.equal(list[3].name, '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; - pollForProcessState(desiredState).then(() => { - // Kill notepad before done - process.kill(list[2].pid); - done(); - }); + pollForProcessTreeSize(term.pid, 4, 500, 5000).then(list => { + assert.equal(list[0].name, 'cmd.exe'); + assert.equal(list[1].name, 'powershell.exe'); + assert.equal(list[2].name, 'notepad.exe'); + assert.equal(list[3].name, '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; + pollForProcessState(desiredState).then(() => { + // Kill notepad before done + process.kill(list[2].pid); + done(); }); - }, 1000); + }); }); });