forward signals to children

lib/run-script-pkg.js

@@ -2,6 +2,7 @@ const makeSpawnArgs = require('./make-spawn-args.js')
 const promiseSpawn = require('@npmcli/promise-spawn')
 const packageEnvs = require('./package-envs.js')
 const { isNodeGypPackage, defaultGypInstallScript } = require('@npmcli/node-gyp')
+const signalManager = require('./signal-manager.js')
 
 // you wouldn't like me when I'm angry...
 const bruce = (id, event, cmd) => `\n> ${id ? id + ' ' : ''}${event}\n> ${cmd}\n`
@@ -57,8 +58,13 @@ const runScriptPkg = async options => {
     pkgid: pkg._id,
     path,
   })
+
+  if (stdio === 'inherit')
+    signalManager.add(p.process)
+
   if (p.stdin)
     p.stdin.end()
+
   return p
 }
 

lib/signal-manager.js

@@ -0,0 +1,47 @@
+const runningProcs = new Set()
+let handlersInstalled = false
+
+const forwardedSignals = [
+  'SIGINT',
+  'SIGTERM'
+]
+
+const handleSignal = signal => {
+  for (const proc of runningProcs) {
+    proc.kill(signal)
+  }
+}
+
+const setupListeners = () => {
+  for (const signal of forwardedSignals) {
+    process.on(signal, handleSignal)
+  }
+  handlersInstalled = true
+}
+
+const cleanupListeners = () => {
+  if (runningProcs.size === 0) {
+    for (const signal of forwardedSignals) {
+      process.removeListener(signal, handleSignal)
+    }
+    handlersInstalled = false
+  }
+}
+
+const add = proc => {
+  runningProcs.add(proc)
+  if (!handlersInstalled)
+    setupListeners()
+
+  proc.once('exit', ({ code }) => {
+    process.exitCode = process.exitCode || code
+    runningProcs.delete(proc)
+    cleanupListeners()
+  })
+}
+
+module.exports = {
+  add,
+  handleSignal,
+  forwardedSignals
+}

package.json

@@ -29,7 +29,7 @@
   },
   "dependencies": {
     "@npmcli/node-gyp": "^1.0.0",
-    "@npmcli/promise-spawn": "^1.2.0",
+    "@npmcli/promise-spawn": "^1.3.0",
     "infer-owner": "^1.0.4",
     "node-gyp": "^7.1.0",
     "read-package-json-fast": "^1.1.3"

test/run-script-pkg.js

@@ -1,11 +1,16 @@
+const { EventEmitter } = require('events')
 const t = require('tap')
 const requireInject = require('require-inject')
 
 let fakeIsNodeGypPackage = false
 
 const runScriptPkg = requireInject('../lib/run-script-pkg.js', {
   '../lib/make-spawn-args.js': options => ['sh', ['-c', options.cmd], options],
-  '@npmcli/promise-spawn': async (...args) => args,
+  '@npmcli/promise-spawn': (...args) => {
+    const p = Promise.resolve(args)
+    p.process = new EventEmitter()
+    return p
+  },
   '@npmcli/node-gyp': {
     isNodeGypPackage: async(path) => Promise.resolve(fakeIsNodeGypPackage),
     defaultGypInstallScript: 'node-gyp rebuild'}
@@ -295,6 +300,7 @@ t.test('end stdin if present', async t => {
     '@npmcli/promise-spawn': (...args) => {
       const p = Promise.resolve(args)
       p.stdin = { end: () => stdinEnded = true }
+      p.process = new EventEmitter()
       return p
     },
   })

test/signal-manager.js

@@ -0,0 +1,56 @@
+const { EventEmitter } = require('events')
+const { test } = require('tap')
+
+const signalManager = require('../lib/signal-manager')
+
+test('adds only one handler for each signal, removes handlers when children have exited', t => {
+  const procOne = new EventEmitter()
+  const procTwo = new EventEmitter()
+
+  for (const signal of signalManager.forwardedSignals) {
+    t.equal(process.listeners(signal).includes(signalManager.handleSignal), false, 'does not have a listener yet')
+  }
+  signalManager.add(procOne)
+
+  for (const signal of signalManager.forwardedSignals) {
+    t.equal(process.listeners(signal).includes(signalManager.handleSignal), true, 'has a listener for forwarded signals')
+  }
+
+  signalManager.add(procTwo)
+  for (const signal of signalManager.forwardedSignals) {
+    const handlers = process.listeners(signal).filter((fn) => fn === signalManager.handleSignal)
+    t.equal(handlers.length, 1, 'only has one handler')
+  }
+
+  procOne.emit('exit', { code: 0 })
+
+  for (const signal of signalManager.forwardedSignals) {
+    t.equal(process.listeners(signal).includes(signalManager.handleSignal), true, 'did not remove listeners yet')
+  }
+
+  procTwo.emit('exit', { code: 0 })
+
+  for (const signal of signalManager.forwardedSignals) {
+    t.equal(process.listeners(signal).includes(signalManager.handleSignal), false, 'listener has been removed')
+  }
+
+  t.done()
+})
+
+test('forwards signals to child process', t => {
+  const proc = new EventEmitter()
+  proc.kill = (signal) => {
+    t.equal(signal, signalManager.forwardedSignals[0], 'child receives correct signal')
+    proc.emit('exit', { code: 0 })
+    for (const signal of signalManager.forwardedSignals) {
+      t.equal(process.listeners(signal).includes(signalManager.handleSignal), false, 'listener has been removed')
+    }
+    t.done()
+  }
+
+  signalManager.add(proc)
+  // passing the signal name here is necessary to fake the effects of actually receiving the signal
+  // per nodejs documentation signal handlers receive the name of the signal as their first parameter
+  // https://nodejs.org/api/process.html#process_signal_events
+  process.emit(signalManager.forwardedSignals[0], signalManager.forwardedSignals[0])
+})