From 8accb846e8bee9bdfc8ddd75b5a275f5c1ba431d Mon Sep 17 00:00:00 2001
From: Chengzhong Wu <cwu631@bloomberg.net>
Date: Mon, 12 Aug 2024 11:03:46 +0100
Subject: [PATCH] lib: `Symbol.dispose` should be enabled with experimental
 flag

The TC39 explicit resource management proposal is still at stage 3 and
under active development. It must be enabled with an experimental
flag `--experimental-explicit-resource-management`. The flag implies
that the V8 option `--js-explicit-resource-management` to be enabled.

When `--experimental-explicit-resource-management` flag is not set, an
experimental warning is emitted.
---
 doc/api/cli.md                                | 12 ++++
 doc/api/test.md                               |  2 +
 lib/internal/process/pre_execution.js         | 62 +++++++++++++------
 src/node_options.cc                           | 13 ++++
 src/node_options.h                            |  1 +
 test/parallel/test-child-process-destroy.js   |  2 +
 test/parallel/test-dgram-async-dispose.mjs    |  2 +
 .../test-events-add-abort-listener.mjs        |  2 +
 .../test-fs-promises-file-handle-dispose.js   |  2 +
 .../test-http-server-async-dispose.js         |  2 +
 .../test-http2-server-async-dispose.js        |  2 +
 .../test-https-server-async-dispose.js        |  2 +
 .../test-net-server-async-dispose.mjs         |  2 +
 test/parallel/test-runner-mock-timers.js      |  2 +
 test/parallel/test-stream-duplex-destroy.js   |  2 +
 test/parallel/test-stream-readable-dispose.js |  2 +
 .../parallel/test-stream-transform-destroy.js |  2 +
 test/parallel/test-stream-writable-destroy.js |  2 +
 test/parallel/test-timers-dispose.js          |  2 +
 .../test-inspector-open-dispose.mjs           |  2 +
 20 files changed, 102 insertions(+), 18 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index c0dfcd418b96a1..fcae1cd0e13297 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -941,6 +941,14 @@ added: v22.3.0
 
 Enable exposition of [EventSource Web API][] on the global scope.
 
+### `--experimental-explicit-resource-management`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+Use this flag to enable [Explicit Resource Management][] support.
+
 ### `--experimental-import-meta-resolve`
 
 <!-- YAML
@@ -2912,6 +2920,7 @@ one is included in the list below.
 * `--experimental-default-type`
 * `--experimental-detect-module`
 * `--experimental-eventsource`
+* `--experimental-explicit-resource-management`
 * `--experimental-import-meta-resolve`
 * `--experimental-json-modules`
 * `--experimental-loader`
@@ -3364,6 +3373,8 @@ documented here:
 
 ### `--jitless`
 
+### `--js-explicit-resource-management`
+
 ### `--interpreted-frames-native-stack`
 
 ### `--prof`
@@ -3435,6 +3446,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
 [ECMAScript module]: esm.md#modules-ecmascript-modules
 [EventSource Web API]: https://html.spec.whatwg.org/multipage/server-sent-events.html#server-sent-events
 [ExperimentalWarning: `vm.measureMemory` is an experimental feature]: vm.md#vmmeasurememoryoptions
+[Explicit Resource Management]: https://github.com/tc39/proposal-explicit-resource-management
 [File System Permissions]: permissions.md#file-system-permissions
 [Loading ECMAScript modules using `require()`]: modules.md#loading-ecmascript-modules-using-require
 [Module customization hooks]: module.md#customization-hooks
diff --git a/doc/api/test.md b/doc/api/test.md
index b1a8e0030bd43f..fde2d66008901c 100644
--- a/doc/api/test.md
+++ b/doc/api/test.md
@@ -2276,6 +2276,8 @@ mock.timers.reset();
 
 ### `timers[Symbol.dispose]()`
 
+> Stability: 1 - Experimental
+
 Calls `timers.reset()`.
 
 ### `timers.tick([milliseconds])`
diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js
index 5b5fffc681887d..d5869ba1db67fe 100644
--- a/lib/internal/process/pre_execution.js
+++ b/lib/internal/process/pre_execution.js
@@ -12,6 +12,7 @@ const {
   NumberParseInt,
   ObjectDefineProperty,
   ObjectFreeze,
+  ObjectHasOwn,
   String,
   StringPrototypeStartsWith,
   Symbol,
@@ -159,29 +160,54 @@ function prepareExecution(options) {
   return mainEntry;
 }
 
+function defineSymbolDisposePolyfill() {
+  ObjectDefineProperty(Symbol, 'dispose', {
+    __proto__: null,
+    configurable: false,
+    enumerable: false,
+    value: SymbolDispose,
+    writable: false,
+  });
+  ObjectDefineProperty(Symbol, 'asyncDispose', {
+    __proto__: null,
+    configurable: false,
+    enumerable: false,
+    value: SymbolAsyncDispose,
+    writable: false,
+  });
+}
+
 function setupSymbolDisposePolyfill() {
   // TODO(MoLow): Remove this polyfill once Symbol.dispose and Symbol.asyncDispose are available in V8.
-  // eslint-disable-next-line node-core/prefer-primordials
-  if (typeof Symbol.dispose !== 'symbol') {
-    ObjectDefineProperty(Symbol, 'dispose', {
-      __proto__: null,
-      configurable: false,
-      enumerable: false,
-      value: SymbolDispose,
-      writable: false,
-    });
+  if (ObjectHasOwn(Symbol, 'dispose')) {
+    return;
   }
 
-  // eslint-disable-next-line node-core/prefer-primordials
-  if (typeof Symbol.asyncDispose !== 'symbol') {
-    ObjectDefineProperty(Symbol, 'asyncDispose', {
-      __proto__: null,
-      configurable: false,
-      enumerable: false,
-      value: SymbolAsyncDispose,
-      writable: false,
-    });
+  if (getOptionValue('--experimental-explicit-resource-management')) {
+    defineSymbolDisposePolyfill();
+    return;
   }
+
+  ObjectDefineProperty(Symbol, 'dispose', {
+    __proto__: null,
+    configurable: true,
+    enumerable: false,
+    get: () => {
+      emitExperimentalWarning('Symbol.dispose/Symbol.asyncDispose');
+      defineSymbolDisposePolyfill();
+      return SymbolDispose;
+    },
+  });
+  ObjectDefineProperty(Symbol, 'asyncDispose', {
+    __proto__: null,
+    configurable: true,
+    enumerable: false,
+    get: () => {
+      emitExperimentalWarning('Symbol.dispose/Symbol.asyncDispose');
+      defineSymbolDisposePolyfill();
+      return SymbolAsyncDispose;
+    },
+  });
 }
 
 function setupUserModules(forceDefaultLoader = false) {
diff --git a/src/node_options.cc b/src/node_options.cc
index 210e3647d0b7eb..9cf857a463bc5c 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -905,6 +905,19 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
   Implies("--experimental-shadow-realm", "--harmony-shadow-realm");
   Implies("--harmony-shadow-realm", "--experimental-shadow-realm");
   ImpliesNot("--no-harmony-shadow-realm", "--experimental-shadow-realm");
+
+  AddOption("--experimental-explicit-resource-management",
+            "",
+            &PerIsolateOptions::experimental_explicit_resource_management,
+            kAllowedInEnvvar);
+  AddOption("--js-explicit-resource-management", "", V8Option{});
+  Implies("--experimental-explicit-resource-management",
+          "--js-explicit-resource-management");
+  Implies("--js-explicit-resource-management",
+          "--experimental-explicit-resource-management");
+  ImpliesNot("--no-js-explicit-resource-management",
+             "--experimental-explicit-resource-management");
+
   AddOption("--build-snapshot",
             "Generate a snapshot blob when the process exits.",
             &PerIsolateOptions::build_snapshot,
diff --git a/src/node_options.h b/src/node_options.h
index a2ed8b1cd4d1d0..9dd367b9afe591 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -264,6 +264,7 @@ class PerIsolateOptions : public Options {
   bool report_uncaught_exception = false;
   bool report_on_signal = false;
   bool experimental_shadow_realm = false;
+  bool experimental_explicit_resource_management = false;
   std::string report_signal = "SIGUSR2";
   bool build_snapshot = false;
   std::string build_snapshot_config;
diff --git a/test/parallel/test-child-process-destroy.js b/test/parallel/test-child-process-destroy.js
index 50763bb0315e5d..90313b2306bae4 100644
--- a/test/parallel/test-child-process-destroy.js
+++ b/test/parallel/test-child-process-destroy.js
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-dgram-async-dispose.mjs b/test/parallel/test-dgram-async-dispose.mjs
index 919bc832570025..47a98a1cfda813 100644
--- a/test/parallel/test-dgram-async-dispose.mjs
+++ b/test/parallel/test-dgram-async-dispose.mjs
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 import * as common from '../common/index.mjs';
 import assert from 'node:assert';
 import dgram from 'node:dgram';
diff --git a/test/parallel/test-events-add-abort-listener.mjs b/test/parallel/test-events-add-abort-listener.mjs
index 69b6a054e395a1..f7772c5e6ddd95 100644
--- a/test/parallel/test-events-add-abort-listener.mjs
+++ b/test/parallel/test-events-add-abort-listener.mjs
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 import * as common from '../common/index.mjs';
 import * as events from 'node:events';
 import * as assert from 'node:assert';
diff --git a/test/parallel/test-fs-promises-file-handle-dispose.js b/test/parallel/test-fs-promises-file-handle-dispose.js
index 406430eef4a204..0708bad936f8a3 100644
--- a/test/parallel/test-fs-promises-file-handle-dispose.js
+++ b/test/parallel/test-fs-promises-file-handle-dispose.js
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-http-server-async-dispose.js b/test/parallel/test-http-server-async-dispose.js
index 8af11dcb7b0c3c..66369da171aec5 100644
--- a/test/parallel/test-http-server-async-dispose.js
+++ b/test/parallel/test-http-server-async-dispose.js
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-http2-server-async-dispose.js b/test/parallel/test-http2-server-async-dispose.js
index 4782e12e417d27..6576ae90663de1 100644
--- a/test/parallel/test-http2-server-async-dispose.js
+++ b/test/parallel/test-http2-server-async-dispose.js
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-https-server-async-dispose.js b/test/parallel/test-https-server-async-dispose.js
index 93b5cb74bbda11..0826e9e6b2f2a2 100644
--- a/test/parallel/test-https-server-async-dispose.js
+++ b/test/parallel/test-https-server-async-dispose.js
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-net-server-async-dispose.mjs b/test/parallel/test-net-server-async-dispose.mjs
index 08765e9c719ca9..1f02ea63098660 100644
--- a/test/parallel/test-net-server-async-dispose.mjs
+++ b/test/parallel/test-net-server-async-dispose.mjs
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 import * as common from '../common/index.mjs';
 import assert from 'node:assert';
 import net from 'node:net';
diff --git a/test/parallel/test-runner-mock-timers.js b/test/parallel/test-runner-mock-timers.js
index 3e8d2d79ede52b..c433565348b7d1 100644
--- a/test/parallel/test-runner-mock-timers.js
+++ b/test/parallel/test-runner-mock-timers.js
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 'use strict';
 process.env.NODE_TEST_KNOWN_GLOBALS = 0;
 const common = require('../common');
diff --git a/test/parallel/test-stream-duplex-destroy.js b/test/parallel/test-stream-duplex-destroy.js
index 52867384057f59..80dda7b24b86c4 100644
--- a/test/parallel/test-stream-duplex-destroy.js
+++ b/test/parallel/test-stream-duplex-destroy.js
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-stream-readable-dispose.js b/test/parallel/test-stream-readable-dispose.js
index e940bf1688f2bf..7b4e43537a7b4d 100644
--- a/test/parallel/test-stream-readable-dispose.js
+++ b/test/parallel/test-stream-readable-dispose.js
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-stream-transform-destroy.js b/test/parallel/test-stream-transform-destroy.js
index 428bab9ce33fcf..352018636633de 100644
--- a/test/parallel/test-stream-transform-destroy.js
+++ b/test/parallel/test-stream-transform-destroy.js
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-stream-writable-destroy.js b/test/parallel/test-stream-writable-destroy.js
index 05d7932b88c182..dcaa46545c3b34 100644
--- a/test/parallel/test-stream-writable-destroy.js
+++ b/test/parallel/test-stream-writable-destroy.js
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-timers-dispose.js b/test/parallel/test-timers-dispose.js
index a75916b4184651..445c1ef22f45c0 100644
--- a/test/parallel/test-timers-dispose.js
+++ b/test/parallel/test-timers-dispose.js
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/sequential/test-inspector-open-dispose.mjs b/test/sequential/test-inspector-open-dispose.mjs
index d3add569c938c8..59638b532fad9a 100644
--- a/test/sequential/test-inspector-open-dispose.mjs
+++ b/test/sequential/test-inspector-open-dispose.mjs
@@ -1,3 +1,5 @@
+// Flags: --experimental-explicit-resource-management
+
 import * as common from '../common/index.mjs';
 import assert from 'node:assert';
 import net from 'node:net';