fix signal handling

lib/index.js

@@ -5,11 +5,19 @@ const { Runner } = require('./runner');
 const { Logger } = require('./logger');
 const { RunMode, initConfig, showUsage, config } = require('./config');
 const { isInternalError } = require('./error');
+const { InterruptHandler } = require('./interrupt');
 const { version: PACKAGE_VERSION } = require('../package.json');
 
 async function run() {
 	let ok = true;
 	let runner = null;
+	const interrupt = new InterruptHandler();
+	interrupt.install(async () => {
+		if (runner) {
+			await runner.shutdown();
+			runner = null;
+		}
+	});
 	try {
 		await initConfig();
 		switch (config.get('runMode')) {

lib/interrupt.js

@@ -0,0 +1,48 @@
+'use strict';
+
+/**
+ * Test files and their helpers may install signal listeners that don't
+ * terminate the process, which disables Node's default behavior and makes
+ * the runner ignore Ctrl-C. This handler is installed before test files are
+ * loaded and guarantees termination after a best-effort cleanup.
+ */
+class InterruptHandler {
+	constructor({ exit = (signal) => process.kill(process.pid, signal) } = {}) {
+		this._exit = exit;
+		this._shutdown = null;
+		this._handling = false;
+		this._onSignal = this._onSignal.bind(this);
+	}
+
+	install(shutdown = null) {
+		this._shutdown = shutdown;
+		this.uninstall();
+		process.on('SIGINT', this._onSignal);
+		process.on('SIGTERM', this._onSignal);
+	}
+
+	uninstall() {
+		process.removeListener('SIGINT', this._onSignal);
+		process.removeListener('SIGTERM', this._onSignal);
+	}
+
+	async _onSignal(signal) {
+		if (!this._handling) {
+			this._handling = true;
+			console.error(`\nReceived ${signal}, terminating`);
+			try {
+				if (this._shutdown) {
+					await this._shutdown();
+				}
+			} catch {
+				// Best-effort cleanup
+			}
+		}
+		process.removeAllListeners(signal);
+		this._exit(signal);
+	}
+}
+
+module.exports = {
+	InterruptHandler
+};

lib/interrupt.test.js

@@ -0,0 +1,117 @@
+'use strict';
+const { expect } = require('chai');
+const sinon = require('sinon');
+const sinonChai = require('sinon-chai');
+const chai = require('chai');
+chai.use(sinonChai);
+
+const { spawn } = require('child_process');
+const path = require('path');
+
+const { InterruptHandler } = require('./interrupt');
+
+describe('InterruptHandler', () => {
+	let exit;
+	let shutdown;
+	let handler;
+
+	beforeEach(() => {
+		exit = sinon.stub();
+		shutdown = sinon.stub().resolves();
+		handler = new InterruptHandler({ exit });
+		handler.install(shutdown);
+	});
+
+	afterEach(() => {
+		handler.uninstall();
+		sinon.restore();
+	});
+
+	it('exits by re-raising the signal', async () => {
+		await handler._onSignal('SIGINT');
+		expect(exit).to.have.been.calledWith('SIGINT');
+	});
+
+	it('exits even if shutdown throws', async () => {
+		shutdown.rejects(new Error('shutdown failed'));
+		await handler._onSignal('SIGINT');
+		expect(exit).to.have.been.calledWith('SIGINT');
+	});
+
+	it('exits immediately on a second signal during shutdown', async () => {
+		let resolveShutdown = null;
+		shutdown.returns(new Promise(resolve => {
+			resolveShutdown = resolve;
+		}));
+		const first = handler._onSignal('SIGINT');
+		handler._onSignal('SIGINT');
+		resolveShutdown();
+		await first;
+		expect(shutdown).to.have.been.calledOnce;
+		expect(exit.callCount).to.equal(2);
+		expect(exit.alwaysCalledWith('SIGINT')).to.be.true;
+	});
+
+	it('registers listeners on install and removes them on uninstall', () => {
+		const sigintCount = process.listenerCount('SIGINT');
+		const sigtermCount = process.listenerCount('SIGTERM');
+		const h = new InterruptHandler({ exit });
+		h.install(shutdown);
+		expect(process.listenerCount('SIGINT')).to.equal(sigintCount + 1);
+		expect(process.listenerCount('SIGTERM')).to.equal(sigtermCount + 1);
+		h.uninstall();
+		expect(process.listenerCount('SIGINT')).to.equal(sigintCount);
+		expect(process.listenerCount('SIGTERM')).to.equal(sigtermCount);
+	});
+
+	it('does not duplicate listeners when install is called twice', () => {
+		const h = new InterruptHandler({ exit });
+		h.install(shutdown);
+		const sigintCount = process.listenerCount('SIGINT');
+		h.install(shutdown);
+		expect(process.listenerCount('SIGINT')).to.equal(sigintCount);
+		h.uninstall();
+	});
+
+	it('does not throw when uninstall is called without install', () => {
+		const h = new InterruptHandler({ exit });
+		expect(() => h.uninstall()).to.not.throw();
+	});
+
+	describe('process integration', () => {
+		it('terminates the process even when other code swallows SIGINT', async function () {
+			this.timeout(20000);
+			const script = `
+				const { InterruptHandler } = require(${JSON.stringify(path.join(__dirname, 'interrupt'))});
+				const h = new InterruptHandler();
+				h.install(async () => {
+					console.log('runner-cleanup');
+				});
+				process.on('SIGINT', () => {
+					console.log('swallowed');
+				});
+				console.log('ready');
+				setInterval(() => {}, 1000);
+			`;
+			const child = spawn(process.execPath, ['-e', script], { stdio: ['ignore', 'pipe', 'pipe'] });
+			let out = '';
+			child.stdout.on('data', data => {
+				out += data;
+			});
+			await new Promise((resolve, reject) => {
+				child.stdout.on('data', data => {
+					if (String(data).includes('ready')) {
+						resolve();
+					}
+				});
+				child.on('error', reject);
+			});
+			child.kill('SIGINT');
+			const [code, sig] = await new Promise(resolve => child.on('exit', (code, signal) => resolve([code, signal])));
+			expect(code).to.be.null;
+			expect(sig).to.equal('SIGINT');
+			expect(out).to.contain('runner-cleanup');
+			expect(out).to.contain('swallowed');
+		});
+	});
+});

lib/util.js

@@ -21,14 +21,18 @@ async function findDeviceOsDirectory() {
 }
 
 async function execCommand(cmd, args, cwd) {
+	// A command passed to spawn() with the shell option set must be a single
+	// string, otherwise Node emits a deprecation warning (DEP0190)
+	const command = [cmd].concat(args || []).join(' ');
+	const shell = os.platform() === 'win32' ? process.env.comspec : '/bin/bash';
 	return new Promise((resolve, reject) => {
-		const p = spawn(cmd, args, {
+		const p = spawn(command, {
 			stdio: [
 				'ignore', // stdin
 				'pipe', // stdout
 				'pipe' // stderr
 			],
-			shell: os.platform() !== 'win32' ? '/bin/bash' : process.env.comspec,
+			shell,
 			cwd
 		});
 		let exited = false;