child_process: treat already-aborted controller as aborting

Trott · danielleadams · commit 78d4d91e54d3 · 2021-01-12T07:10:23.000-05:00
If an AbortController passed to execfile() is already aborted, use the same behavior as if the controller was aborted after calling execfile(). This mimics the behavior of fetch in the browser. PR-URL: #36644 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
diff --git a/lib/child_process.js b/lib/child_process.js
@@ -376,6 +376,8 @@ function execFile(file /* , args, options, callback */) {
   }
   if (options.signal) {
     if (options.signal.aborted) {
+      if (!ex)
+        ex = new AbortError();
       process.nextTick(() => kill());
     } else {
       const childController = new AbortController();
diff --git a/test/parallel/test-child-process-execfile.js b/test/parallel/test-child-process-execfile.js
@@ -53,25 +53,19 @@ const execOpts = { encoding: 'utf8', shell: true };
   const ac = new AbortController();
   const { signal } = ac;
 
-  const firstCheck = common.mustCall((err) => {
-    assert.strictEqual(err.code, 'ABORT_ERR');
-    assert.strictEqual(err.name, 'AbortError');
-    assert.strictEqual(err.signal, undefined);
-  });
-
-  const secondCheck = common.mustCall((err) => {
-    assert.strictEqual(err.code, null);
-    assert.strictEqual(err.name, 'Error');
-    assert.strictEqual(err.signal, 'SIGTERM');
-  });
-
-  execFile(process.execPath, [echoFixture, 0], { signal }, (err) => {
-    firstCheck(err);
-    // Test that re-using the aborted signal results in immediate SIGTERM.
-    execFile(process.execPath, [echoFixture, 0], { signal }, secondCheck);
-  });
+  const test = () => {
+    const check = common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ABORT_ERR');
+      assert.strictEqual(err.name, 'AbortError');
+      assert.strictEqual(err.signal, undefined);
+    });
+    execFile(process.execPath, [echoFixture, 0], { signal }, check);
+  };
 
+  test();
   ac.abort();
+  // Verify that it still works the same way now that the signal is aborted.
+  test();
 }
 
 {

Original file line number	Diff line number	Diff line change
`@@ -376,6 +376,8 @@ function execFile(file /* , args, options, callback */) {`
`376`	`376`	`}`
`377`	`377`	`if (options.signal) {`
`378`	`378`	`if (options.signal.aborted) {`
	`379`	`+ if (!ex)`
	`380`	`+ ex = new AbortError();`
`379`	`381`	`process.nextTick(() => kill());`
`380`	`382`	`} else {`
`381`	`383`	`const childController = new AbortController();`