lib: avoid using __proto__: null in AbortController/AbortSignal

[jasnell]

Using __proto__: null on objects will properties immediately puts them into the slower DictionaryProperties mode. Using a __proto__: { __proto: null }, ... pattern, however, keeps the objects in FastProperties mode.

Technically this is an observable change since calling Object.getPrototoTypeOf(...) some of these will no longer return null. So a decision needs to be made about whether these types of changes are semver-major or not (/cc @nodejs/tsc @nodejs/performance). Before we go too much further in making these kinds of changes, I'd like to get the semver-major question settled.

[gurgunday]

lgtm

[mcollina]

I don’t think it’s semver-major but I would not backport it either.

[aduh95]

Given this is run only once (and I think even in the snapshot) and not ref'd anywhere, I'm not sure it's worth fighting against the lint rule

[jasnell]

Not planning to fight against the lint rule. I'm in the process of updating the lint rule.

[jasnell]

#66680

[aduh95]

What about the fact that it's run only once when building the startup? Do you know if it's still has some noticeable perf effects, or is it more for consistency sake?

[codecov]

Codecov Report

❌ Patch coverage is 96.29630% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.49%. Comparing base (04bd737) to head (3d05b97).
⚠️ Report is 15 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/abort_controller.js	96.29%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #66678      +/-   ##
==========================================
+ Coverage   90.47%   90.49%   +0.01%     
==========================================
  Files         791      791              
  Lines      277097   277103       +6     
  Branches    53277    53268       -9     
==========================================
+ Hits       250696   250754      +58     
+ Misses      16813    16764      -49     
+ Partials     9588     9585       -3     

Files with missing lines	Coverage Δ
lib/internal/abort_controller.js	95.52% <96.29%> (+0.11%)	⬆️

... and 42 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[aduh95]

I suggest the following diff, which I suspect has the same performance:

diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js
index 8bb2ee687ba..e39d7b4d843 100644
--- a/lib/internal/abort_controller.js
+++ b/lib/internal/abort_controller.js
@@ -8,7 +8,6 @@ const {
   ObjectAssign,
   ObjectDefineProperties,
   ObjectDefineProperty,
-  ObjectFreeze,
   ObjectSetPrototypeOf,
   PromiseResolve,
   PromiseWithResolvers,
@@ -74,15 +73,6 @@ const {
 
 let _MessageChannel;
 
-// An object literal with __proto__: null and properties will be
-// created as slow Dictionary properties. An object literal with
-// __proto__: null and no properties is fast mode. An object
-// literal with { __proto__: kNullProtoObject, ... } and
-// properties will be fast mode. It's a V8 optimization quirk.
-// To ensure that object literals are created in fast mode,
-// use __proto__: kNullProtoObject.
-const kNullProtoObject = ObjectFreeze({ __proto__: null });
-
 const kDontThrowSymbol = Symbol('kDontThrowSymbol');
 
 // Loading the MessageChannel and markTransferable have to be done lazily
@@ -307,7 +297,7 @@ class AbortSignal extends EventTarget {
    */
   static abort(
     reason = new DOMException('This operation was aborted', 'AbortError')) {
-    return new AbortSignal(kDontThrowSymbol, { __proto__: kNullProtoObject, aborted: true, reason });
+    return new AbortSignal(kDontThrowSymbol, { __proto__: kEmptyObject, aborted: true, reason });
   }
 
   /**
@@ -332,10 +322,10 @@ class AbortSignal extends EventTarget {
   static any(signals) {
     const signalsArray = converters['sequence<AbortSignal>'](
       signals,
-      { __proto__: kNullProtoObject, context: 'signals' },
+      { __proto__: kEmptyObject, context: 'signals' },
     );
 
-    const resultSignal = new AbortSignal(kDontThrowSymbol, { __proto__: kNullProtoObject, composite: true });
+    const resultSignal = new AbortSignal(kDontThrowSymbol, { __proto__: kEmptyObject, composite: true });
     if (!signalsArray.length) {
       return resultSignal;
     }
@@ -435,7 +425,7 @@ class AbortSignal extends EventTarget {
     if (aborted) {
       const reason = this.reason;
       return {
-        data: { __proto__: kNullProtoObject, aborted, reason },
+        data: { __proto__: kEmptyObject, aborted, reason },
         deserializeInfo: 'internal/abort_controller:ClonedAbortSignal',
       };
     }
@@ -446,10 +436,10 @@ class AbortSignal extends EventTarget {
     this.addEventListener('abort', () => {
       port1.postMessage(this.reason);
       port1.close();
-    }, { __proto__: kNullProtoObject, once: true });
+    }, { __proto__: kEmptyObject, once: true });
 
     return {
-      data: { __proto__: kNullProtoObject, port: port2 },
+      data: { __proto__: kEmptyObject, port: port2 },
       deserializeInfo: 'internal/abort_controller:ClonedAbortSignal',
     };
   }
@@ -489,7 +479,7 @@ class AbortSignal extends EventTarget {
 converters['sequence<AbortSignal>'] = createSequenceConverter(converters.AbortSignal);
 
 function ClonedAbortSignal() {
-  return new AbortSignal(kDontThrowSymbol, { __proto__: kNullProtoObject, transferable: true });
+  return new AbortSignal(kDontThrowSymbol, { __proto__: kEmptyObject, transferable: true });
 }
 ClonedAbortSignal.prototype[kDeserialize] = () => {};
 
@@ -497,12 +487,8 @@ ObjectDefineProperties(AbortSignal.prototype, {
   aborted: kEnumerableProperty,
 });
 
-// TODO(@jasnell): the kNullProtoObject is a null-prototype object but it is
-// not recognized as such by the esline rule, which will need to be updated
-// to recognize this pattern.
-// eslint-disable-next-line node-core/avoid-prototype-pollution
 ObjectDefineProperty(AbortSignal.prototype, SymbolToStringTag, {
-  __proto__: kNullProtoObject,
+  __proto__: null,
   writable: false,
   enumerable: false,
   configurable: true,
@@ -604,7 +590,7 @@ class AbortController {
 
   static [kMakeTransferable]() {
     const controller = new AbortController();
-    controller.#signal = new AbortSignal(kDontThrowSymbol, { __proto__: kNullProtoObject, transferable: true });
+    controller.#signal = new AbortSignal(kDontThrowSymbol, { __proto__: kEmptyObject, transferable: true });
     return controller;
   }
 }
@@ -635,12 +621,12 @@ function transferableAbortController() {
  * @returns {Promise<void>}
  */
 async function aborted(signal, resource) {
-  converters.AbortSignal(signal, { __proto__: kNullProtoObject, context: 'signal' });
+  converters.AbortSignal(signal, { __proto__: kEmptyObject, context: 'signal' });
   validateObject(resource, 'resource', kValidateObjectAllowObjects);
   if (signal.aborted)
     return PromiseResolve();
   const abortPromise = PromiseWithResolvers();
-  const opts = { __proto__: kNullProtoObject, [kWeakHandler]: resource, once: true, [kResistStopPropagation]: true };
+  const opts = { __proto__: kEmptyObject, [kWeakHandler]: resource, once: true, [kResistStopPropagation]: true };
   signal.addEventListener('abort', abortPromise.resolve, opts);
   return abortPromise.promise;
 }
@@ -650,9 +636,8 @@ ObjectDefineProperties(AbortController.prototype, {
   abort: kEnumerableProperty,
 });
 
-// eslint-disable-next-line node-core/avoid-prototype-pollution
 ObjectDefineProperty(AbortController.prototype, SymbolToStringTag, {
-  __proto__: kNullProtoObject,
+  __proto__: null,
   writable: false,
   enumerable: false,
   configurable: true,