From 6c8fa6e3765a1cdc047fd2d565ab241c05734f98 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Sat, 11 Jun 2022 20:30:51 -0500
Subject: [PATCH] lib: refactor transferable AbortSignal

The use of makeTransferable on all AbortSignal instances made creating them always slow,
causing bottlenecks in stuff like Web Streams. This refactors that so that AbortSignals
are not transferable by default, which is actually the correct standard behavior anyway.
A new transferableAbortSignal and transferableAbortController utility is provided to
enable the transferable use case.

Signed-off-by: James M Snell <jasnell@gmail.com>
---
 doc/api/util.md                             | 27 +++++++++++++
 lib/internal/abort_controller.js            | 45 +++++++++++++++++++--
 lib/internal/worker/js_transferable.js      |  2 +
 lib/util.js                                 | 13 ++++++
 test/parallel/test-abortsignal-cloneable.js | 11 +++--
 5 files changed, 91 insertions(+), 7 deletions(-)

diff --git a/doc/api/util.md b/doc/api/util.md
index 9d5201adfcc9a7..5683c6fb864ab7 100644
--- a/doc/api/util.md
+++ b/doc/api/util.md
@@ -1470,6 +1470,33 @@ Returns the `string` after replacing any surrogate code points
 (or equivalently, any unpaired surrogate code units) with the
 Unicode "replacement character" U+FFFD.
 
+## `util.transferableAbortController()`
+
+<!--
+added: REPLACEME
+-->
+
+Creates and returns an {AbortController} instance whose {AbortSignal} is marked
+as transferable and can be used with `structuredClone()` or `postMessage()`.
+
+## `util.transferableAbortSignal(signal)`
+
+<!--
+added: REPLACEME
+-->
+
+* `signal` {AbortSignal}
+* Returns: {AbortSignal}
+
+Marks the given {AbortSignal} as transferable so that it can be used with
+`structuredClone()` and `postMessage()`.
+
+```cjs
+const signal = transferableAbortSignal(AbortSignal.timeout(100));
+const channel = new MessageChannel();
+channel.port2.postMessage(signal, [signal]);
+```
+
 ## `util.types`
 
 <!-- YAML
diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js
index e4f5f14f1e4cbe..19fb4711d2b037 100644
--- a/lib/internal/abort_controller.js
+++ b/lib/internal/abort_controller.js
@@ -9,6 +9,7 @@ const {
   ObjectSetPrototypeOf,
   ObjectDefineProperty,
   SafeFinalizationRegistry,
+  ReflectConstruct,
   SafeSet,
   Symbol,
   SymbolToStringTag,
@@ -31,6 +32,7 @@ const { inspect } = require('internal/util/inspect');
 const {
   codes: {
     ERR_ILLEGAL_CONSTRUCTOR,
+    ERR_INVALID_ARG_TYPE,
     ERR_INVALID_THIS,
   }
 } = require('internal/errors');
@@ -159,7 +161,7 @@ class AbortSignal extends EventTarget {
    */
   static abort(
     reason = new DOMException('This operation was aborted', 'AbortError')) {
-    return createAbortSignal(true, reason);
+    return createAbortSignal({ aborted: true, reason });
   }
 
   /**
@@ -256,7 +258,7 @@ class AbortSignal extends EventTarget {
 }
 
 function ClonedAbortSignal() {
-  return createAbortSignal();
+  return createAbortSignal({ transferable: true });
 }
 ClonedAbortSignal.prototype[kDeserialize] = () => {};
 
@@ -274,12 +276,25 @@ ObjectDefineProperty(AbortSignal.prototype, SymbolToStringTag, {
 
 defineEventHandler(AbortSignal.prototype, 'abort');
 
-function createAbortSignal(aborted = false, reason = undefined) {
+/**
+ * @param {{
+ *   aborted? : boolean,
+ *   reason? : any,
+ *   transferable? : boolean
+ * }} [init]
+ * @returns {AbortSignal}
+ */
+function createAbortSignal(init) {
+  const {
+    aborted = false,
+    reason = undefined,
+    transferable = false,
+  } = init;
   const signal = new EventTarget();
   ObjectSetPrototypeOf(signal, AbortSignal.prototype);
   signal[kAborted] = aborted;
   signal[kReason] = reason;
-  return lazyMakeTransferable(signal);
+  return transferable ? lazyMakeTransferable(signal) : signal;
 }
 
 function abortSignal(signal, reason) {
@@ -329,6 +344,26 @@ class AbortController {
   }
 }
 
+/**
+ * Enables the AbortSignal to be transferable using structuredClone/postMessage.
+ * @param {AbortSignal} signal
+ * @returns {AbortSignal}
+ */
+function transferableAbortSignal(signal) {
+  if (signal?.[kAborted] === undefined)
+    throw new ERR_INVALID_ARG_TYPE('signal', 'AbortSignal', signal);
+  return makeTransferable(signal);
+}
+
+/**
+ * Creates an AbortController with a transferable AbortSignal
+ */
+function transferableAbortController() {
+  return ReflectConstruct(function() {
+    this[kSignal] = createAbortSignal({ transferable: true });
+  }, [], AbortController);
+}
+
 ObjectDefineProperties(AbortController.prototype, {
   signal: kEnumerableProperty,
   abort: kEnumerableProperty,
@@ -347,4 +382,6 @@ module.exports = {
   AbortController,
   AbortSignal,
   ClonedAbortSignal,
+  transferableAbortSignal,
+  transferableAbortController,
 };
diff --git a/lib/internal/worker/js_transferable.js b/lib/internal/worker/js_transferable.js
index 337413f0258a7e..a94893cfe77aa1 100644
--- a/lib/internal/worker/js_transferable.js
+++ b/lib/internal/worker/js_transferable.js
@@ -40,6 +40,8 @@ function setup() {
 }
 
 function makeTransferable(obj) {
+  // If the object is already transferable, skip all this.
+  if (obj instanceof JSTransferable) return obj;
   const inst = ReflectConstruct(JSTransferable, [], obj.constructor);
   const properties = ObjectGetOwnPropertyDescriptors(obj);
   const propertiesValues = ObjectValues(properties);
diff --git a/lib/util.js b/lib/util.js
index 78e6b807ee5ef6..909426ed4c0785 100644
--- a/lib/util.js
+++ b/lib/util.js
@@ -78,6 +78,13 @@ const {
   toUSVString,
 } = require('internal/util');
 
+let abortController;
+
+function lazyAbortController() {
+  abortController ??= require('internal/abort_controller');
+  return abortController;
+}
+
 let internalDeepEqual;
 
 /**
@@ -377,5 +384,11 @@ module.exports = {
   toUSVString,
   TextDecoder,
   TextEncoder,
+  get transferableAbortSignal() {
+    return lazyAbortController().transferableAbortSignal;
+  },
+  get transferableAbortController() {
+    return lazyAbortController().transferableAbortController;
+  },
   types
 };
diff --git a/test/parallel/test-abortsignal-cloneable.js b/test/parallel/test-abortsignal-cloneable.js
index e88af333fe9dd4..a5061a7b81b65b 100644
--- a/test/parallel/test-abortsignal-cloneable.js
+++ b/test/parallel/test-abortsignal-cloneable.js
@@ -3,6 +3,11 @@
 const common = require('../common');
 const { ok, strictEqual } = require('assert');
 const { setImmediate: pause } = require('timers/promises');
+const {
+  transferableAbortSignal,
+  transferableAbortController,
+} = require('util');
+
 
 function deferred() {
   let res;
@@ -11,7 +16,7 @@ function deferred() {
 }
 
 (async () => {
-  const ac = new AbortController();
+  const ac = transferableAbortController();
   const mc = new MessageChannel();
 
   const deferred1 = deferred();
@@ -54,7 +59,7 @@ function deferred() {
 })().then(common.mustCall());
 
 {
-  const signal = AbortSignal.abort('boom');
+  const signal = transferableAbortSignal(AbortSignal.abort('boom'));
   ok(signal.aborted);
   strictEqual(signal.reason, 'boom');
   const mc = new MessageChannel();
@@ -70,7 +75,7 @@ function deferred() {
 {
   // The cloned AbortSignal does not keep the event loop open
   // waiting for the abort to be triggered.
-  const ac = new AbortController();
+  const ac = transferableAbortController();
   const mc = new MessageChannel();
   mc.port1.onmessage = common.mustCall();
   mc.port2.postMessage(ac.signal, [ac.signal]);