From 8d4592920fe2cf38e90d4f459821783a7a10b05c Mon Sep 17 00:00:00 2001
From: Nitzan Uziely <nitzan@testim.io>
Date: Sun, 7 Feb 2021 01:39:54 +0200
Subject: [PATCH] child_process: add timeout and killSignal to spawn and fork

Add support for timeout and killSignal to spawn and fork.

Fixes: https://github.com/nodejs/node/issues/27639
---
 doc/api/child_process.md                      | 14 ++++
 lib/child_process.js                          | 42 ++++++++++-
 .../test-child-process-fork-abort-signal.js   | 17 ++++-
 ...-child-process-fork-timeout-kill-signal.js | 69 ++++++++++++++++++
 .../test-child-process-spawn-controller.js    | 46 ++++++++++++
 ...child-process-spawn-timeout-kill-signal.js | 70 +++++++++++++++++++
 6 files changed, 256 insertions(+), 2 deletions(-)
 create mode 100644 test/parallel/test-child-process-fork-timeout-kill-signal.js
 create mode 100644 test/parallel/test-child-process-spawn-timeout-kill-signal.js

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index 87a62ddbbc864e..054b0d11504a1b 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -351,6 +351,9 @@ controller.abort();
 <!-- YAML
 added: v0.5.0
 changes:
+  - version: REPLACEME
+    pr-url: xxx
+    description: killSignal and timeout was added.
   - version: v15.6.0
     pr-url: https://github.com/nodejs/node/pull/36603
     description: AbortSignal support was added.
@@ -394,6 +397,10 @@ changes:
   * `uid` {number} Sets the user identity of the process (see setuid(2)).
   * `windowsVerbatimArguments` {boolean} No quoting or escaping of arguments is
     done on Windows. Ignored on Unix. **Default:** `false`.
+  * `timeout` {number} In milliseconds the maximum amount of time the process
+    is allowed to run. **Default:** `undefined`.
+  * `killSignal` {string|integer} The signal value to be used when the spawned
+    process will be killed by timeout or abort signal. **Default:** `'SIGTERM'`.
 * Returns: {ChildProcess}
 
 The `child_process.fork()` method is a special case of
@@ -431,6 +438,9 @@ The `signal` option works exactly the same way it does in
 <!-- YAML
 added: v0.1.90
 changes:
+  - version: REPLACEME
+    pr-url: xxx
+    description: killSignal and timeout were added.
   - version: v15.5.0
     pr-url: https://github.com/nodejs/node/pull/36432
     description: AbortSignal support was added.
@@ -477,6 +487,10 @@ changes:
   * `windowsHide` {boolean} Hide the subprocess console window that would
     normally be created on Windows systems. **Default:** `false`.
   * `signal` {AbortSignal} allows aborting the execFile using an AbortSignal.
+  * `timeout` {number} In milliseconds the maximum amount of time the process
+    is allowed to run. **Default:** `undefined`.
+  * `killSignal` {string|integer} The signal value to be used when the spawned
+    process will be killed by timeout or abort signal. **Default:** `'SIGTERM'`.
 
 * Returns: {ChildProcess}
 
diff --git a/lib/child_process.js b/lib/child_process.js
index 74699a16834c30..5d29d3bc2ee91c 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -141,7 +141,26 @@ function fork(modulePath /* , args, options */) {
   options.execPath = options.execPath || process.execPath;
   options.shell = false;
 
-  return spawn(options.execPath, args, options);
+  validateTimeout(options.timeout);
+  if (options.killSignal) {
+    options.killSignal = sanitizeKillSignal(options.killSignal);
+  }
+  const child = spawn(options.execPath, args, options);
+
+  if (options.timeout > 0) {
+    let timeoutId = setTimeout(() => {
+      child.kill(options.killSignal);
+      timeoutId = null;
+    }, options.timeout);
+
+    child.once('close', () => {
+      if (timeoutId) {
+        clearTimeout(timeoutId);
+        timeoutId = null;
+      }
+    });
+  }
+  return child;
 }
 
 function _forkChild(fd, serializationMode) {
@@ -758,6 +777,12 @@ function spawnWithSignal(file, args, options) {
   const opts = options && typeof options === 'object' && ('signal' in options) ?
     { ...options, signal: undefined } :
     options;
+
+  // Validate the timeout, if present.
+  validateTimeout(options?.timeout);
+  if (options?.killSignal) {
+    options.killSignal = sanitizeKillSignal(options.killSignal);
+  }
   const child = spawn(file, args, opts);
 
   if (options && options.signal) {
@@ -777,6 +802,21 @@ function spawnWithSignal(file, args, options) {
       child.once('close', remove);
     }
   }
+
+  if (options?.timeout > 0) {
+    let timeoutId = setTimeout(() => {
+      child.kill(options.killSignal);
+      timeoutId = null;
+    }, options.timeout);
+
+    child.once('close', () => {
+      if (timeoutId) {
+        clearTimeout(timeoutId);
+        timeoutId = null;
+      }
+    });
+  }
+
   return child;
 }
 module.exports = {
diff --git a/test/parallel/test-child-process-fork-abort-signal.js b/test/parallel/test-child-process-fork-abort-signal.js
index 63367479234d7f..b453b18d365620 100644
--- a/test/parallel/test-child-process-fork-abort-signal.js
+++ b/test/parallel/test-child-process-fork-abort-signal.js
@@ -26,8 +26,23 @@ const { fork } = require('child_process');
   const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
     signal
   });
-  cp.on('exit', mustCall());
+  cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGTERM')));
+  cp.on('error', mustCall((err) => {
+    strictEqual(err.name, 'AbortError');
+  }));
+}
+
+{
+  // Test correct signal sent
+  const ac = new AbortController();
+  const { signal } = ac;
+  const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
+    signal,
+    killSignal: 'SIGKILL',
+  });
+  cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGKILL')));
   cp.on('error', mustCall((err) => {
     strictEqual(err.name, 'AbortError');
   }));
+  process.nextTick(() => ac.abort());
 }
diff --git a/test/parallel/test-child-process-fork-timeout-kill-signal.js b/test/parallel/test-child-process-fork-timeout-kill-signal.js
new file mode 100644
index 00000000000000..e993bbb5503ecc
--- /dev/null
+++ b/test/parallel/test-child-process-fork-timeout-kill-signal.js
@@ -0,0 +1,69 @@
+'use strict';
+
+const { mustCall } = require('../common');
+const { strictEqual, ok, throws } = require('assert');
+const fixtures = require('../common/fixtures');
+const { fork } = require('child_process');
+const { getEventListeners } = require('events');
+
+{
+  // Verify default signal + closes after at least 4 ms.
+  let closed = false;
+  setTimeout(() => ok(!closed), 4);
+  const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
+    timeout: 5,
+  });
+  cp.on('close', mustCall(() => closed = true));
+  cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGTERM')));
+}
+
+{
+  // Verify correct signal + closes after at least 4 ms.
+  let closed = false;
+  setTimeout(() => ok(!closed), 4);
+  const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
+    timeout: 5,
+    killSignal: 'SIGKILL',
+  });
+  cp.on('close', mustCall(() => closed = true));
+  cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGKILL')));
+}
+
+{
+  // Verify timeout verification
+  throws(() => fork(fixtures.path('child-process-stay-alive-forever.js'), {
+    timeout: 'badValue',
+  }), /ERR_OUT_OF_RANGE/);
+
+  throws(() => fork(fixtures.path('child-process-stay-alive-forever.js'), {
+    timeout: {},
+  }), /ERR_OUT_OF_RANGE/);
+}
+
+{
+  // Verify abort signal gets unregistered
+  const signal = new EventTarget();
+  signal.aborted = false;
+
+  const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
+    timeout: 6,
+    signal,
+  });
+  strictEqual(getEventListeners(signal, 'abort').length, 1);
+  cp.on('close', mustCall(() => {
+    strictEqual(getEventListeners(signal, 'abort').length, 0);
+  }));
+}
+
+{
+  // Verify no error happens when abort is called timeout
+  const controller = new AbortController();
+  const { signal } = controller;
+
+  const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
+    timeout: 100,
+    signal,
+  });
+  cp.on('error', mustCall());
+  setTimeout(() => controller.abort(), 1);
+}
diff --git a/test/parallel/test-child-process-spawn-controller.js b/test/parallel/test-child-process-spawn-controller.js
index c25307907c1b26..098095a85414ea 100644
--- a/test/parallel/test-child-process-spawn-controller.js
+++ b/test/parallel/test-child-process-spawn-controller.js
@@ -39,3 +39,49 @@ const cp = require('child_process');
     assert.strictEqual(e.name, 'AbortError');
   }));
 }
+
+{
+  // Verify that passing kill signal works.
+  const controller = new AbortController();
+  const { signal } = controller;
+
+  const echo = cp.spawn('echo', ['fun'], {
+    encoding: 'utf8',
+    shell: true,
+    signal,
+    killSignal: 'SIGKILL',
+  });
+
+  echo.on('close', common.mustCall((code, killSignal) => {
+    assert.strictEqual(killSignal, 'SIGKILL');
+  }));
+
+  echo.on('error', common.mustCall((e) => {
+    assert.strictEqual(e.name, 'AbortError');
+  }));
+
+  controller.abort();
+}
+
+{
+  // Verify that passing a different kill signal works.
+  const controller = new AbortController();
+  const { signal } = controller;
+
+  const echo = cp.spawn('echo', ['fun'], {
+    encoding: 'utf8',
+    shell: true,
+    signal,
+    killSignal: 'SIGTERM',
+  });
+
+  echo.on('close', common.mustCall((code, killSignal) => {
+    assert.strictEqual(killSignal, 'SIGTERM');
+  }));
+
+  echo.on('error', common.mustCall((e) => {
+    assert.strictEqual(e.name, 'AbortError');
+  }));
+
+  controller.abort();
+}
diff --git a/test/parallel/test-child-process-spawn-timeout-kill-signal.js b/test/parallel/test-child-process-spawn-timeout-kill-signal.js
new file mode 100644
index 00000000000000..267ded0094c043
--- /dev/null
+++ b/test/parallel/test-child-process-spawn-timeout-kill-signal.js
@@ -0,0 +1,70 @@
+'use strict';
+
+const { mustCall } = require('../common');
+const { strictEqual, ok, throws } = require('assert');
+const fixtures = require('../common/fixtures');
+const { spawn } = require('child_process');
+const { getEventListeners } = require('events');
+
+const aliveForeverFile = 'child-process-stay-alive-forever.js';
+{
+  // Verify default signal + closes after at least 4 ms.
+  let closed = false;
+  setTimeout(() => ok(!closed), 4);
+  const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+    timeout: 5,
+  });
+  cp.on('close', mustCall(() => closed = true));
+  cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGTERM')));
+}
+
+{
+  // Verify correct signal + closes after at least 4 ms.
+  let closed = false;
+  setTimeout(() => ok(!closed), 4);
+  const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+    timeout: 5,
+    killSignal: 'SIGKILL',
+  });
+  cp.on('close', mustCall(() => closed = true));
+  cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGKILL')));
+}
+
+{
+  // Verify timeout verification
+  throws(() => spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+    timeout: 'badValue',
+  }), /ERR_OUT_OF_RANGE/);
+
+  throws(() => spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+    timeout: {},
+  }), /ERR_OUT_OF_RANGE/);
+}
+
+{
+  // Verify abort signal gets unregistered
+  const signal = new EventTarget();
+  signal.aborted = false;
+
+  const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+    timeout: 6,
+    signal,
+  });
+  strictEqual(getEventListeners(signal, 'abort').length, 1);
+  cp.on('close', mustCall(() => {
+    strictEqual(getEventListeners(signal, 'abort').length, 0);
+  }));
+}
+
+{
+  // Verify no error happens when abort is called timeout
+  const controller = new AbortController();
+  const { signal } = controller;
+
+  const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+    timeout: 100,
+    signal,
+  });
+  cp.on('error', mustCall());
+  setTimeout(() => controller.abort(), 1);
+}