Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 46 additions & 24 deletions lib/diagnostics_channel.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Comment thread
Renegade334 marked this conversation as resolved.

// Enter stores using withScope
if (activeChannel._stores) {
Expand All @@ -109,19 +109,27 @@ 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();
// 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]();
}
}
}
}

Expand Down Expand Up @@ -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]();
}
}
}

Expand Down Expand Up @@ -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]();
}
}
}

Expand Down Expand Up @@ -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;
Expand All @@ -531,6 +544,8 @@ class TracingChannel {
context.error = err;
error.publish(context);
throw err;
} finally {
scope[SymbolDispose]();
}
}

Expand All @@ -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) {
Expand All @@ -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.
Expand All @@ -595,6 +610,8 @@ class TracingChannel {
context.error = err;
error.publish(context);
throw err;
} finally {
scope[SymbolDispose]();
}
}

Expand All @@ -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]();
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions test/parallel/test-diagnostics-channel-no-erm.js
Original file line number Diff line number Diff line change
@@ -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(() => {});
Loading