diagnostics_channel: replace using with try-finally

lib/diagnostics_channel.js

@@ -23,6 +23,7 @@ const {
   codes: {
     ERR_INVALID_ARG_TYPE,
   },
+  NodeAggregateError,
 } = require('internal/errors');
 const {
   validateFunction,
@@ -88,8 +89,8 @@ class RunStoresScope {
   #stack;
 
   constructor(activeChannel, data) {
-    // eslint-disable-next-line no-restricted-globals
-    using stack = new DisposableStack();
+    // TODO: use native DisposableStack once supported in core
+    const stack = [];
 
     // Enter stores using withScope
     if (activeChannel._stores) {
@@ -109,19 +110,39 @@ class RunStoresScope {
           }
         }
 
-        stack.use(store.withScope(newContext));
+        ArrayPrototypePush(stack, store.withScope(newContext));
       }
     }
 
     // Publish data
     activeChannel.publish(data);
 
     // Transfer ownership of the stack
-    this.#stack = stack.move();
+    this.#stack = stack;
   }
 
   [SymbolDispose]() {
-    this.#stack[SymbolDispose]();
+    if (this.#stack !== undefined) {
+      const stack = this.#stack;
+      this.#stack = undefined;
+
+      const errors = [];
+      for (let i = stack.length - 1; i >= 0; i--) {
+        try {
+          stack[i][SymbolDispose]();
+        } catch (error) {
+          ArrayPrototypePush(errors, error);
+        }
+      }
+
+      if (errors.length === 0) {
+        return;
+      } else if (errors.length === 1) {
+        throw errors[0];
+      } else {
+        throw new NodeAggregateError(errors);
+      }
+    }
   }
 }
 
@@ -197,9 +218,12 @@ class ActiveChannel {
   }
 
   runStores(data, fn, thisArg, ...args) {
-    // eslint-disable-next-line no-unused-vars
-    using scope = this.withStoreScope(data);
-    return ReflectApply(fn, thisArg, args);
+    const scope = this.withStoreScope(data);
+    try {
+      return ReflectApply(fn, thisArg, args);
+    } finally {
+      scope[SymbolDispose]();
+    }
   }
 }
 
@@ -396,9 +420,12 @@ class BoundedChannel {
 
   run(context, fn, thisArg, ...args) {
     context ??= {};
-    // eslint-disable-next-line no-unused-vars
-    using scope = this.withScope(context);
-    return ReflectApply(fn, thisArg, args);
+    const scope = this.withScope(context);
+    try {
+      return ReflectApply(fn, thisArg, args);
+    } finally {
+      scope[SymbolDispose]();
+    }
   }
 }
 
@@ -521,16 +548,19 @@ class TracingChannel {
 
     const { error } = this;
 
-    // eslint-disable-next-line no-unused-vars
-    using scope = this.#callWindow.withScope(context);
+    const scope = this.#callWindow.withScope(context);
     try {
-      const result = ReflectApply(fn, thisArg, args);
-      context.result = result;
-      return result;
-    } catch (err) {
-      context.error = err;
-      error.publish(context);
-      throw err;
+      try {
+        const result = ReflectApply(fn, thisArg, args);
+        context.result = result;
+        return result;
+      } catch (err) {
+        context.error = err;
+        error.publish(context);
+        throw err;
+      }
+    } finally {
+      scope[SymbolDispose]();
     }
   }
 
@@ -550,9 +580,12 @@ class TracingChannel {
       context.error = err;
       error.publish(context);
       // Use continuation window for asyncStart/asyncEnd
-      // eslint-disable-next-line no-unused-vars
-      using scope = continuationWindow.withScope(context);
-      // TODO: Is there a way to have asyncEnd _after_ the continuation?
+      const scope = continuationWindow.withScope(context);
+      try {
+        // TODO: Is there a way to have asyncEnd _after_ the continuation?
+      } finally {
+        scope[SymbolDispose]();
+      }
     }
 
     function onRejectWithRethrow(err) {
@@ -563,38 +596,44 @@ class TracingChannel {
     function onResolve(result) {
       context.result = result;
       // Use continuation window for asyncStart/asyncEnd
-      // eslint-disable-next-line no-unused-vars
-      using scope = continuationWindow.withScope(context);
-      // TODO: Is there a way to have asyncEnd _after_ the continuation?
-      return result;
+      const scope = continuationWindow.withScope(context);
+      try {
+        // TODO: Is there a way to have asyncEnd _after_ the continuation?
+        return result;
+      } finally {
+        scope[SymbolDispose]();
+      }
     }
 
-    // eslint-disable-next-line no-unused-vars
-    using scope = this.#callWindow.withScope(context);
+    const scope = this.#callWindow.withScope(context);
     try {
-      const result = ReflectApply(fn, thisArg, args);
-      // If the return value is not a thenable, return it directly with a warning.
-      // Do not publish to asyncStart/asyncEnd.
-      if (typeof result?.then !== 'function') {
-        emitNonThenableWarning(fn);
-        context.result = result;
+      try {
+        const result = ReflectApply(fn, thisArg, args);
+        // If the return value is not a thenable, return it directly with a warning.
+        // Do not publish to asyncStart/asyncEnd.
+        if (typeof result?.then !== 'function') {
+          emitNonThenableWarning(fn);
+          context.result = result;
+          return result;
+        }
+        // isPromise() matches sub-classes, but we need to match only direct
+        // instances of the native Promise type to safely use PromisePrototypeThen.
+        if (isPromise(result) && ObjectGetPrototypeOf(result) === PromisePrototype) {
+          return PromisePrototypeThen(result, onResolve, onRejectWithRethrow);
+        }
+        // For non-native thenables, subscribe to the result but return the
+        // original thenable so the consumer can continue handling it directly.
+        // Non-native thenables don't have unhandledRejection tracking, so
+        // swallowing the rejection here doesn't change existing behaviour.
+        result.then(onResolve, onReject);
         return result;
+      } catch (err) {
+        context.error = err;
+        error.publish(context);
+        throw err;
       }
-      // isPromise() matches sub-classes, but we need to match only direct
-      // instances of the native Promise type to safely use PromisePrototypeThen.
-      if (isPromise(result) && ObjectGetPrototypeOf(result) === PromisePrototype) {
-        return PromisePrototypeThen(result, onResolve, onRejectWithRethrow);
-      }
-      // For non-native thenables, subscribe to the result but return the
-      // original thenable so the consumer can continue handling it directly.
-      // Non-native thenables don't have unhandledRejection tracking, so
-      // swallowing the rejection here doesn't change existing behaviour.
-      result.then(onResolve, onReject);
-      return result;
-    } catch (err) {
-      context.error = err;
-      error.publish(context);
-      throw err;
+    } finally {
+      scope[SymbolDispose]();
     }
   }
 
@@ -615,23 +654,29 @@ class TracingChannel {
       }
 
       // Use continuation window for asyncStart/asyncEnd around callback
-      // eslint-disable-next-line no-unused-vars
-      using scope = continuationWindow.withScope(context);
-      return ReflectApply(callback, this, arguments);
+      const scope = continuationWindow.withScope(context);
+      try {
+        return ReflectApply(callback, this, arguments);
+      } finally {
+        scope[SymbolDispose]();
+      }
     }
 
     const callback = ArrayPrototypeAt(args, position);
     validateFunction(callback, 'callback');
     ArrayPrototypeSplice(args, position, 1, wrappedCallback);
 
-    // eslint-disable-next-line no-unused-vars
-    using scope = this.#callWindow.withScope(context);
+    const scope = this.#callWindow.withScope(context);
     try {
-      return ReflectApply(fn, thisArg, args);
-    } catch (err) {
-      context.error = err;
-      error.publish(context);
-      throw err;
+      try {
+        return ReflectApply(fn, thisArg, args);
+      } catch (err) {
+        context.error = err;
+        error.publish(context);
+        throw err;
+      }
+    } finally {
+      scope[SymbolDispose]();
     }
   }
 }

test/parallel/test-diagnostics-channel-no-erm.js

@@ -0,0 +1,20 @@
+'use strict';
+require('../common');
+const assert = require('node:assert');
+const { spawnSync } = require('node:child_process');
+
+// This test ensures that using diagnostics_channel does not cause a segfault
+// when explicit resource management is disabled in V8.
+// Refs: https://github.com/nodejs/node/issues/64230
+
+const child = spawnSync(process.execPath, [
+  '--no-js-explicit-resource-management',
+  '--eval',
+  'require("diagnostics_channel").tracingChannel("foo").traceSync(() => {})'
+]);
+
+// If it segfaults, signal will be something like 'SIGSEGV'.
+// We assert that the process exited cleanly (status 0).
+assert.strictEqual(child.signal, null);
+assert.strictEqual(child.status, 0);
+assert.strictEqual(child.stderr.toString(), '');