Skip to content

Commit d7d9082

Browse files
diagnostics_channel: replace using with try-finally
Signed-off-by: Ayush Chaudhary <ayush23chaudhary@gmail.com>
1 parent 42fd49a commit d7d9082

2 files changed

Lines changed: 56 additions & 24 deletions

File tree

lib/diagnostics_channel.js

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ class RunStoresScope {
8888
#stack;
8989

9090
constructor(activeChannel, data) {
91-
// eslint-disable-next-line no-restricted-globals
92-
using stack = new DisposableStack();
91+
// TODO: use native DisposableStack once supported in core
92+
const stack = [];
9393

9494
// Enter stores using withScope
9595
if (activeChannel._stores) {
@@ -109,19 +109,26 @@ class RunStoresScope {
109109
}
110110
}
111111

112-
stack.use(store.withScope(newContext));
112+
ArrayPrototypePush(stack, store.withScope(newContext));
113113
}
114114
}
115115

116116
// Publish data
117117
activeChannel.publish(data);
118118

119119
// Transfer ownership of the stack
120-
this.#stack = stack.move();
120+
this.#stack = stack;
121121
}
122122

123123
[SymbolDispose]() {
124-
this.#stack[SymbolDispose]();
124+
if (this.#stack !== undefined) {
125+
const stack = this.#stack;
126+
this.#stack = undefined;
127+
128+
for (let i = stack.length - 1; i >= 0; i--) {
129+
stack[i][SymbolDispose]();
130+
}
131+
}
125132
}
126133
}
127134

@@ -197,9 +204,12 @@ class ActiveChannel {
197204
}
198205

199206
runStores(data, fn, thisArg, ...args) {
200-
// eslint-disable-next-line no-unused-vars
201-
using scope = this.withStoreScope(data);
202-
return ReflectApply(fn, thisArg, args);
207+
const scope = this.withStoreScope(data);
208+
try {
209+
return ReflectApply(fn, thisArg, args);
210+
} finally {
211+
scope[SymbolDispose]();
212+
}
203213
}
204214
}
205215

@@ -396,9 +406,12 @@ class BoundedChannel {
396406

397407
run(context, fn, thisArg, ...args) {
398408
context ??= {};
399-
// eslint-disable-next-line no-unused-vars
400-
using scope = this.withScope(context);
401-
return ReflectApply(fn, thisArg, args);
409+
const scope = this.withScope(context);
410+
try {
411+
return ReflectApply(fn, thisArg, args);
412+
} finally {
413+
scope[SymbolDispose]();
414+
}
402415
}
403416
}
404417

@@ -520,9 +533,8 @@ class TracingChannel {
520533
}
521534

522535
const { error } = this;
536+
const scope = this.#callWindow.withScope(context);
523537

524-
// eslint-disable-next-line no-unused-vars
525-
using scope = this.#callWindow.withScope(context);
526538
try {
527539
const result = ReflectApply(fn, thisArg, args);
528540
context.result = result;
@@ -531,6 +543,8 @@ class TracingChannel {
531543
context.error = err;
532544
error.publish(context);
533545
throw err;
546+
} finally {
547+
scope[SymbolDispose]();
534548
}
535549
}
536550

@@ -550,9 +564,9 @@ class TracingChannel {
550564
context.error = err;
551565
error.publish(context);
552566
// Use continuation window for asyncStart/asyncEnd
553-
// eslint-disable-next-line no-unused-vars
554-
using scope = continuationWindow.withScope(context);
567+
const scope = continuationWindow.withScope(context);
555568
// TODO: Is there a way to have asyncEnd _after_ the continuation?
569+
scope[SymbolDispose]();
556570
}
557571

558572
function onRejectWithRethrow(err) {
@@ -563,14 +577,14 @@ class TracingChannel {
563577
function onResolve(result) {
564578
context.result = result;
565579
// Use continuation window for asyncStart/asyncEnd
566-
// eslint-disable-next-line no-unused-vars
567-
using scope = continuationWindow.withScope(context);
580+
const scope = continuationWindow.withScope(context);
568581
// TODO: Is there a way to have asyncEnd _after_ the continuation?
582+
scope[SymbolDispose]();
569583
return result;
570584
}
571585

572-
// eslint-disable-next-line no-unused-vars
573-
using scope = this.#callWindow.withScope(context);
586+
const scope = this.#callWindow.withScope(context);
587+
574588
try {
575589
const result = ReflectApply(fn, thisArg, args);
576590
// If the return value is not a thenable, return it directly with a warning.
@@ -595,6 +609,8 @@ class TracingChannel {
595609
context.error = err;
596610
error.publish(context);
597611
throw err;
612+
} finally {
613+
scope[SymbolDispose]();
598614
}
599615
}
600616

@@ -615,23 +631,28 @@ class TracingChannel {
615631
}
616632

617633
// Use continuation window for asyncStart/asyncEnd around callback
618-
// eslint-disable-next-line no-unused-vars
619-
using scope = continuationWindow.withScope(context);
620-
return ReflectApply(callback, this, arguments);
634+
const scope = continuationWindow.withScope(context);
635+
try {
636+
return ReflectApply(callback, this, arguments);
637+
} finally {
638+
scope[SymbolDispose]();
639+
}
621640
}
622641

623642
const callback = ArrayPrototypeAt(args, position);
624643
validateFunction(callback, 'callback');
625644
ArrayPrototypeSplice(args, position, 1, wrappedCallback);
626645

627-
// eslint-disable-next-line no-unused-vars
628-
using scope = this.#callWindow.withScope(context);
646+
const scope = this.#callWindow.withScope(context);
647+
629648
try {
630649
return ReflectApply(fn, thisArg, args);
631650
} catch (err) {
632651
context.error = err;
633652
error.publish(context);
634653
throw err;
654+
} finally {
655+
scope[SymbolDispose]();
635656
}
636657
}
637658
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Flags: --no-js-explicit-resource-management
2+
'use strict';
3+
4+
require('../common');
5+
const { tracingChannel } = require('diagnostics_channel');
6+
7+
// This test ensures that using diagnostics_channel does not cause a segfault
8+
// when explicit resource management is disabled in V8.
9+
// Refs: https://github.com/nodejs/node/issues/64230
10+
11+
tracingChannel('foo').traceSync(() => {});

0 commit comments

Comments
 (0)