From ddacd902282f751fca448de9c51f8d133e337472 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 to spawn and fork

Add support for timeout to spawn and fork.

Fixes: https://github.com/nodejs/node/issues/27639
---
 doc/api/child_process.md                      | 16 +++++-
 lib/child_process.js                          | 26 ++++++++--
 ...-child-process-fork-timeout-kill-signal.js | 50 +++++++++++++++++++
 ...child-process-spawn-timeout-kill-signal.js | 50 +++++++++++++++++++
 4 files changed, 135 insertions(+), 7 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 b7fce216282012..5c3a8d34f050c9 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: https://github.com/nodejs/node/pull/37256
+    description: timeout was added.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/37325
     description: killSignal for AbortSignal was added.
@@ -399,6 +402,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
@@ -436,6 +443,9 @@ The `signal` option works exactly the same way it does in
 <!-- YAML
 added: v0.1.90
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37256
+    description: timeout was added.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/37325
     description: killSignal for AbortSignal was added.
@@ -485,8 +495,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.
-  * `killSignal` {string} The signal value to be used when the spawned
-    process will be killed by the abort signal. **Default:** `'SIGTERM'`.
+  * `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 57b5eaa71b1bb5..b026b8275e633f 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -599,15 +599,14 @@ function abortChildProcess(child, killSignal) {
 
 
 function spawn(file, args, options) {
-  const child = new ChildProcess();
   options = normalizeSpawnArguments(file, args, options);
+  validateTimeout(options.timeout, 'options.timeout');
+  validateAbortSignal(options.signal, 'options.signal');
+  const killSignal = sanitizeKillSignal(options.killSignal);
+  const child = new ChildProcess();
 
   if (options.signal) {
     const signal = options.signal;
-    // Validate signal, if present
-    validateAbortSignal(signal, 'options.signal');
-    const killSignal = sanitizeKillSignal(options.killSignal);
-    // Do nothing and throw if already aborted
     if (signal.aborted) {
       onAbortListener();
     } else {
@@ -626,6 +625,22 @@ function spawn(file, args, options) {
   debug('spawn', options);
   child.spawn(options);
 
+  if (options.timeout > 0) {
+    let timeoutId = setTimeout(() => {
+      if (timeoutId) {
+        child.kill(killSignal);
+        timeoutId = null;
+      }
+    }, options.timeout);
+
+    child.once('exit', () => {
+      if (timeoutId) {
+        clearTimeout(timeoutId);
+        timeoutId = null;
+      }
+    });
+  }
+
   return child;
 }
 
@@ -790,6 +805,7 @@ function spawnWithSignal(file, args, options) {
   }
   return child;
 }
+
 module.exports = {
   _forkChild,
   ChildProcess,
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..ef08d4b12ab94a
--- /dev/null
+++ b/test/parallel/test-child-process-fork-timeout-kill-signal.js
@@ -0,0 +1,50 @@
+'use strict';
+
+const { mustCall } = require('../common');
+const { strictEqual, throws } = require('assert');
+const fixtures = require('../common/fixtures');
+const { fork } = require('child_process');
+const { getEventListeners } = require('events');
+
+{
+  // Verify default signal
+  const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
+    timeout: 5,
+  });
+  cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGTERM')));
+}
+
+{
+  // Verify correct signal + closes after at least 4 ms.
+  const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
+    timeout: 5,
+    killSignal: 'SIGKILL',
+  });
+  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('exit', mustCall(() => {
+    strictEqual(getEventListeners(signal, 'abort').length, 0);
+  }));
+}
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..59a974d0e0b448
--- /dev/null
+++ b/test/parallel/test-child-process-spawn-timeout-kill-signal.js
@@ -0,0 +1,50 @@
+'use strict';
+
+const { mustCall } = require('../common');
+const { strictEqual, 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
+  const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+    timeout: 5,
+  });
+  cp.on('exit', mustCall((code, ks) => strictEqual(ks, 'SIGTERM')));
+}
+
+{
+  // Verify SIGKILL signal + closes
+  const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+    timeout: 6,
+    killSignal: 'SIGKILL',
+  });
+  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 controller = new AbortController();
+  const { signal } = controller;
+  const cp = spawn(process.execPath, [fixtures.path(aliveForeverFile)], {
+    timeout: 6,
+    signal,
+  });
+  strictEqual(getEventListeners(signal, 'abort').length, 1);
+  cp.on('exit', mustCall(() => {
+    strictEqual(getEventListeners(signal, 'abort').length, 0);
+  }));
+}