From be4e7e5e416e7f181eb344396f6b61803dcb1da1 Mon Sep 17 00:00:00 2001 From: stelcheck Date: Thu, 7 Sep 2017 17:03:36 +0900 Subject: [PATCH] forward signals correctly signals are passed to the spawned child process; this means that the main process should not react to them, and instead wait for the spawned process to exit as a result of receiving the forwarded signal. --- src/bin.ts | 45 ++++++++++++++++++++++++++++++++++++--------- src/index.spec.ts | 29 ++++++++++++++++++++++++++++- tests/signals.ts | 8 ++++++++ 3 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 tests/signals.ts diff --git a/src/bin.ts b/src/bin.ts index 2aa928fab..4054af4f7 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -54,16 +54,43 @@ v8flags(function (err, v8flags) { const proc = spawn( process.execPath, nodeArgs.concat(join(__dirname, '_bin.js'), scriptArgs), - { stdio: 'inherit' } + { + // We need to run in detached mode so to avoid + // automatic propagation of signals to the child process. + // This is necessary because by default, keyboard interrupts + // are propagated to the process tree, but `kill` is not. + // + // See: https://nodejs.org/api/child_process.html#child_process_options_detached + detached: true, + + // Pipe all input and output to this process + stdio: 'inherit' + } ) - proc.on('exit', function (code: number, signal: string) { - process.on('exit', function () { - if (signal) { - process.kill(process.pid, signal) - } else { - process.exit(code) - } - }) + // Ignore signals, and instead forward them to the + // child process + const forward = (signal: string) => process.on(signal, () => proc.kill(signal)) + + // Interrupt (CTRL-C) + forward('SIGINT') + + // Termination (`kill` default signal) + forward('SIGTERM') + + // Terminal size change must be forwarded to the subprocess + forward('SIGWINCH') + + // On exit, exit this process with the same exit code + proc.on('close', (code: number, signal: string) => { + if (signal) { + process.kill(process.pid, signal) + } else if (code) { + process.exit(code) + } }) + + // If this process is exited with any other signals, + // kill the subprocess with the same signal + process.on('exit', (_code: number, signal: string) => proc.kill(signal)) }) diff --git a/src/index.spec.ts b/src/index.spec.ts index 839fb4f40..745bc0148 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -1,5 +1,5 @@ import { expect } from 'chai' -import { exec } from 'child_process' +import { exec, spawn } from 'child_process' import { join } from 'path' import semver = require('semver') import ts = require('typescript') @@ -20,6 +20,33 @@ describe('ts-node', function () { }) describe('cli', function () { + this.slow(1000) + + it('should forward signals to the child process', function (done) { + this.slow(5000) + + const proc = spawn('node', [ + EXEC_PATH, + 'tests/signals' + ], { + shell: '/bin/bash' + }) + + let stdout = '' + proc.stdout.on('data', (data) => stdout += data.toString()) + + proc.on('exit', function (code) { + expect(code).to.equal(0) + expect(stdout).to.equal('exited\n') + + return done() + }) + + // Leave enough time for node to fully start + // the process, then send a signal + setTimeout(() => proc.kill('SIGINT'), 1000) + }) + it('should execute cli', function (done) { exec(`${BIN_EXEC} tests/hello-world`, function (err, stdout) { expect(err).to.equal(null) diff --git a/tests/signals.ts b/tests/signals.ts new file mode 100644 index 000000000..0463080c9 --- /dev/null +++ b/tests/signals.ts @@ -0,0 +1,8 @@ +process.on('SIGINT', () => { + setTimeout(() => { + console.log('exited') + process.exit(0) + }, 500) +}) + +setInterval(() => console.log('should not be reached'), 3000)