fix: gracefully close vscode on sigint (#208)

Right now, if the test process is killed, VS Code can remain open. Instead tear it down gracefully.

Author: connor4312

.husky/pre-commit

@@ -1,5 +1,9 @@
 # Changelog
 
+### 2.3.1 | 2022-04-04
+
+- Gracefully kill VS Code if SIGINT is received
+
 ### 2.3.0 | 2022-02-27
 
 - Automatically use the most recent version matching `engines.vscode` in extensions' package.json

CHANGELOG.md

@@ -0,0 +1,13 @@
+#!/bin/sh
+
+ROOT_PID=$1
+SIGNAL=$2
+
+terminateTree() {
+	for cpid in $(/usr/bin/pgrep -P $1); do
+		terminateTree $cpid
+	done
+	kill -$SIGNAL $1 > /dev/null 2>&1
+}
+
+terminateTree $ROOT_PID

killTree.sh

@@ -7,6 +7,7 @@ import * as cp from 'child_process';
 import * as path from 'path';
 import { downloadAndUnzipVSCode, DownloadVersion, DownloadPlatform, defaultCachePath } from './download';
 import { ProgressReporter } from './progress';
+import { killTree } from './util';
 
 export interface TestOptions {
 	/**
@@ -171,16 +172,37 @@ function hasArg(argName: string, argList: readonly string[]) {
 	return argList.some((a) => a === `--${argName}` || a.startsWith(`--${argName}=`));
 }
 
+const SIGINT = 'SIGINT';
+
 async function innerRunTests(
 	executable: string,
 	args: string[],
 	testRunnerEnv?: {
 		[key: string]: string | undefined;
 	}
 ): Promise<number> {
-	return new Promise<number>((resolve, reject) => {
-		const fullEnv = Object.assign({}, process.env, testRunnerEnv);
-		const cmd = cp.spawn(executable, args, { env: fullEnv });
+	const fullEnv = Object.assign({}, process.env, testRunnerEnv);
+	const cmd = cp.spawn(executable, args, { env: fullEnv });
+	let exitRequested = false;
+	const ctrlc1 = () => {
+		process.removeListener(SIGINT, ctrlc1);
+		process.on(SIGINT, ctrlc2);
+		console.log('Closing VS Code gracefully. Press Ctrl+C to force close.');
+		exitRequested = true;
+		cmd.kill(SIGINT); // this should cause the returned promise to resolve
+	};
+
+	const ctrlc2 = () => {
+		console.log('Closing VS Code forcefully.');
+		process.removeListener(SIGINT, ctrlc2);
+		exitRequested = true;
+		killTree(cmd.pid!, true);
+	};
+
+	const prom = new Promise<number>((resolve, reject) => {
+		if (cmd.pid) {
+			process.on(SIGINT, ctrlc1);
+		}
 
 		cmd.stdout.on('data', (d) => process.stdout.write(d));
 		cmd.stderr.on('data', (d) => process.stderr.write(d));
@@ -213,7 +235,21 @@ async function innerRunTests(
 		}
 
 		cmd.on('close', onProcessClosed);
-
 		cmd.on('exit', onProcessClosed);
 	});
+
+	let code: number;
+	try {
+		code = await prom;
+	} finally {
+		process.removeListener(SIGINT, ctrlc1);
+		process.removeListener(SIGINT, ctrlc2);
+	}
+
+	// exit immediately if we handled a SIGINT and no one else did
+	if (exitRequested && process.listenerCount(SIGINT) === 0) {
+		process.exit(1);
+	}
+
+	return code;
 }

lib/runTest.ts

@@ -3,15 +3,16 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { ChildProcess, spawn } from 'child_process';
+import { readFileSync } from 'fs';
+import * as createHttpProxyAgent from 'http-proxy-agent';
+import * as https from 'https';
+import * as createHttpsProxyAgent from 'https-proxy-agent';
 import * as path from 'path';
 import { URL } from 'url';
-import * as https from 'https';
-import * as request from './request';
 import { DownloadPlatform } from './download';
-import * as createHttpsProxyAgent from 'https-proxy-agent';
-import * as createHttpProxyAgent from 'http-proxy-agent';
-import { readFileSync } from 'fs';
-import { getProfileArguments, TestOptions } from './runTest';
+import * as request from './request';
+import { TestOptions, getProfileArguments } from './runTest';
 
 export let systemDefaultPlatform: DownloadPlatform;
 
@@ -27,8 +28,8 @@ switch (process.platform) {
 			process.arch === 'arm64'
 				? 'win32-arm64-archive'
 				: process.arch === 'ia32'
-				? 'win32-archive'
-				: 'win32-x64-archive';
+					? 'win32-archive'
+					: 'win32-x64-archive';
 		break;
 	default:
 		systemDefaultPlatform =
@@ -232,3 +233,31 @@ export function onceWithoutRejections<T, Args extends unknown[]>(fn: (...args: A
 		return value;
 	};
 }
+
+export function killTree(processId: number, force: boolean) {
+	let cp: ChildProcess;
+
+	if (process.platform === 'win32') {
+		const windir = process.env['WINDIR'] || 'C:\\Windows';
+
+		// when killing a process in Windows its child processes are *not* killed but become root processes.
+		// Therefore we use TASKKILL.EXE
+		cp = spawn(path.join(windir, 'System32', 'taskkill.exe'), [
+			...force ? ['/F'] : [],
+			'/T',
+			'/PID',
+			processId.toString(),
+		], { stdio: 'inherit' });
+	} else {
+		// on linux and OS X we kill all direct and indirect child processes as well
+		cp = spawn('sh', [
+			path.resolve(__dirname, '../killTree.sh'),
+			processId.toString(),
+			force ? '9' : '15',
+		], { stdio: 'inherit' });
+	}
+
+	return new Promise<void>((resolve, reject) => {
+		cp.on('error', reject).on('exit', resolve);
+	});
+}

lib/util.ts

@@ -25,7 +25,7 @@
 		"vscode:prepublish": "yarn run compile",
 		"compile": "tsc -p ./",
 		"watch": "tsc -watch -p ./",
-		"test": "tsc -p ./ && node ./out/sample/test/runTest.js"
+		"test": "cd .. && tsc && cd sample tsc && node ./out/test/runTest.js"
 	},
 	"devDependencies": {
 		"@types/glob": "^7.1.3",

sample/package.json

@@ -1,7 +1,7 @@
 import * as path from 'path';
 import * as cp from 'child_process';
 
-import { runTests, downloadAndUnzipVSCode, resolveCliArgsFromVSCodeExecutablePath } from '../../lib/index';
+import { runTests, downloadAndUnzipVSCode, resolveCliArgsFromVSCodeExecutablePath } from '../../..';
 
 async function go() {
 	try {
@@ -17,7 +17,7 @@ async function go() {
 		});
 
 		const extensionTestsPath2 = path.resolve(__dirname, './suite2');
-		const testWorkspace = path.resolve(__dirname, '../../../test-fixtures/fixture1');
+		const testWorkspace = path.resolve(__dirname, '../../src/test-fixtures/fixture1');
 
 		/**
 		 * Running another test suite on a specific workspace

sample/test-fixtures/fixture1/foo.js renamed to sample/src/test-fixtures/fixture1/foo.js

@@ -8,8 +8,7 @@
 		"skipLibCheck": true
 	},
 	"include": [
-    "src",
-    "test"
+    "src"
 	],
   "exclude": ["node_modules", ".vscode-test"]
 }