From 385e8e8d7bbfaf12c57254cfa7f37ee771d0f11b Mon Sep 17 00:00:00 2001
From: Benjamin Gruenbaum <benjamingr@gmail.com>
Date: Tue, 22 Dec 2020 15:27:14 +0200
Subject: [PATCH] child_process: support AbortSignal in fork

PR-URL: https://github.com/nodejs/node/pull/36603
Backport-PR-URL: https://github.com/nodejs/node/pull/38386
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
---
 doc/api/child_process.md                      | 11 ++++--
 lib/child_process.js                          |  4 +--
 .../child-process-stay-alive-forever.js       |  3 ++
 .../test-child-process-fork-abort-signal.js   | 34 +++++++++++++++++++
 4 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100644 test/fixtures/child-process-stay-alive-forever.js
 create mode 100644 test/parallel/test-child-process-fork-abort-signal.js

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index 16e88401e4d7bb..13c342b07e765f 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/36603
+    description: AbortSignal support was added.
   - version:
       - v13.2.0
       - v12.16.0
@@ -375,9 +378,11 @@ changes:
   * `execPath` {string} Executable used to create the child process.
   * `execArgv` {string[]} List of string arguments passed to the executable.
     **Default:** `process.execArgv`.
+  * `gid` {number} Sets the group identity of the process (see setgid(2)).
   * `serialization` {string} Specify the kind of serialization used for sending
     messages between processes. Possible values are `'json'` and `'advanced'`.
     See [Advanced serialization][] for more details. **Default:** `'json'`.
+  * `signal` {AbortSignal} Allows closing the subprocess using an AbortSignal.
   * `silent` {boolean} If `true`, stdin, stdout, and stderr of the child will be
     piped to the parent, otherwise they will be inherited from the parent, see
     the `'pipe'` and `'inherit'` options for [`child_process.spawn()`][]'s
@@ -386,10 +391,9 @@ changes:
     When this option is provided, it overrides `silent`. If the array variant
     is used, it must contain exactly one item with value `'ipc'` or an error
     will be thrown. For instance `[0, 1, 2, 'ipc']`.
+  * `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`.
-  * `uid` {number} Sets the user identity of the process (see setuid(2)).
-  * `gid` {number} Sets the group identity of the process (see setgid(2)).
 * Returns: {ChildProcess}
 
 The `child_process.fork()` method is a special case of
@@ -420,6 +424,9 @@ current process.
 The `shell` option available in [`child_process.spawn()`][] is not supported by
 `child_process.fork()` and will be ignored if set.
 
+The `signal` option works exactly the same way it does in
+[`child_process.spawn()`][].
+
 ### `child_process.spawn(command[, args][, options])`
 <!-- YAML
 added: v0.1.90
diff --git a/lib/child_process.js b/lib/child_process.js
index 3249c97a021cfd..fae9c930ebb0bf 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -127,7 +127,7 @@ function fork(modulePath /* , args, options */) {
   options.execPath = options.execPath || process.execPath;
   options.shell = false;
 
-  return spawn(options.execPath, args, options);
+  return spawnWithSignal(options.execPath, args, options);
 }
 
 function _forkChild(fd, serializationMode) {
@@ -719,7 +719,7 @@ function spawnWithSignal(file, args, options) {
     validateAbortSignal(options.signal, 'options.signal');
     function kill() {
       if (child._handle) {
-        child._handle.kill('SIGTERM');
+        child.kill('SIGTERM');
         child.emit('error', new AbortError());
       }
     }
diff --git a/test/fixtures/child-process-stay-alive-forever.js b/test/fixtures/child-process-stay-alive-forever.js
new file mode 100644
index 00000000000000..fda313eba074c1
--- /dev/null
+++ b/test/fixtures/child-process-stay-alive-forever.js
@@ -0,0 +1,3 @@
+setInterval(() => {
+    // Starting an interval to stay alive.
+}, 1000);
diff --git a/test/parallel/test-child-process-fork-abort-signal.js b/test/parallel/test-child-process-fork-abort-signal.js
new file mode 100644
index 00000000000000..809821738d1b9d
--- /dev/null
+++ b/test/parallel/test-child-process-fork-abort-signal.js
@@ -0,0 +1,34 @@
+// Flags: --experimental-abortcontroller
+'use strict';
+
+const { mustCall } = require('../common');
+const { strictEqual } = require('assert');
+const fixtures = require('../common/fixtures');
+const { fork } = require('child_process');
+
+{
+  // Test aborting a forked child_process after calling fork
+  const ac = new AbortController();
+  const { signal } = ac;
+  const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
+    signal
+  });
+  cp.on('exit', mustCall());
+  cp.on('error', mustCall((err) => {
+    strictEqual(err.name, 'AbortError');
+  }));
+  process.nextTick(() => ac.abort());
+}
+{
+  // Test passing an already aborted signal to a forked child_process
+  const ac = new AbortController();
+  const { signal } = ac;
+  ac.abort();
+  const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), {
+    signal
+  });
+  cp.on('exit', mustCall());
+  cp.on('error', mustCall((err) => {
+    strictEqual(err.name, 'AbortError');
+  }));
+}