From 50b7999334b890e3a455b2c5b8607f0774639e86 Mon Sep 17 00:00:00 2001 From: Ayush Chaudhary Date: Thu, 2 Jul 2026 21:50:14 +0530 Subject: [PATCH] diagnostics_channel: replace using with try-finally Signed-off-by: Ayush Chaudhary --- lib/diagnostics_channel.js | 70 ++++++++++++------- .../test-diagnostics-channel-no-erm.js | 11 +++ 2 files changed, 57 insertions(+), 24 deletions(-) create mode 100644 test/parallel/test-diagnostics-channel-no-erm.js diff --git a/lib/diagnostics_channel.js b/lib/diagnostics_channel.js index 7b78851208df66..f0ad14cfb54346 100644 --- a/lib/diagnostics_channel.js +++ b/lib/diagnostics_channel.js @@ -88,8 +88,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,7 +109,7 @@ class RunStoresScope { } } - stack.use(store.withScope(newContext)); + ArrayPrototypePush(stack, store.withScope(newContext)); } } @@ -117,11 +117,19 @@ class RunStoresScope { activeChannel.publish(data); // Transfer ownership of the stack - this.#stack = stack.move(); + // TODO: Restore stack.move() when DisposableStack is restored in core to avoid broken behavior. + this.#stack = stack; } [SymbolDispose]() { - this.#stack[SymbolDispose](); + if (this.#stack !== undefined) { + const stack = this.#stack; + this.#stack = undefined; + + for (let i = stack.length - 1; i >= 0; i--) { + stack[i][SymbolDispose](); + } + } } } @@ -197,9 +205,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 +407,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](); + } } } @@ -520,9 +534,8 @@ class TracingChannel { } const { error } = this; + const scope = this.#callWindow.withScope(context); - // eslint-disable-next-line no-unused-vars - using scope = this.#callWindow.withScope(context); try { const result = ReflectApply(fn, thisArg, args); context.result = result; @@ -531,6 +544,8 @@ class TracingChannel { context.error = err; error.publish(context); throw err; + } finally { + scope[SymbolDispose](); } } @@ -550,9 +565,9 @@ 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); + const scope = continuationWindow.withScope(context); // TODO: Is there a way to have asyncEnd _after_ the continuation? + scope[SymbolDispose](); } function onRejectWithRethrow(err) { @@ -563,14 +578,14 @@ 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); + const scope = continuationWindow.withScope(context); // TODO: Is there a way to have asyncEnd _after_ the continuation? + scope[SymbolDispose](); return result; } - // 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. @@ -595,6 +610,8 @@ class TracingChannel { context.error = err; error.publish(context); throw err; + } finally { + scope[SymbolDispose](); } } @@ -615,23 +632,28 @@ 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; + } finally { + scope[SymbolDispose](); } } } diff --git a/test/parallel/test-diagnostics-channel-no-erm.js b/test/parallel/test-diagnostics-channel-no-erm.js new file mode 100644 index 00000000000000..636bf28523b26b --- /dev/null +++ b/test/parallel/test-diagnostics-channel-no-erm.js @@ -0,0 +1,11 @@ +// Flags: --no-js-explicit-resource-management +'use strict'; + +require('../common'); +const { tracingChannel } = require('diagnostics_channel'); + +// 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 + +tracingChannel('foo').traceSync(() => {});