Skip to content

Commit 6f0a888

Browse files
committed
fix: also memoize sub-channels of native tracingChannel(name)
On Node versions where the GC bug exists AND tracingChannel is native (Node 16.17+/18.7+ through 20.5), tracingChannel(name) creates its five sub-channels by calling Node's internal channel() registry directly — bypassing dc.channel's memoization. So a publisher capturing tracingChannel(name).start in a closure can end up with a different Channel than a subscriber attaching via dc.channel( 'tracing:name:start').subscribe(...) for the same name. Wrap dc.tracingChannel on those versions to construct the TracingChannel via the {start, end, ...} object form, supplying memoized sub-channels from dc.channel. Native tracingChannel accepts this form, so the result is a real native TracingChannel with stable sub-channel identity. The wrap is applied unconditionally inside this patch (which itself is only loaded when hasGarbageCollectionBug() is true). For !hasTracingChannel versions, the polyfill in patch-tracing-channel.js runs after this and overwrites dc.tracingChannel — that path already captures dc.channel transitively, so the polyfilled tracingChannel inherits the memoization. No coverage gap on either side. Three new test cases pin the contract — all six sub-channel-stability assertions fail on the prior version (channel-only memoization), all sixteen pass with this commit. Object-form tracingChannel input is passed through unchanged.
1 parent deac894 commit 6f0a888

2 files changed

Lines changed: 93 additions & 0 deletions

File tree

patch-garbage-collection-bug.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const {
99

1010
module.exports = function(unpatched) {
1111
const dc_channel = unpatched.channel;
12+
const dc_tracingChannel = unpatched.tracingChannel;
1213
const channels = new WeakSet();
1314
const byName = new Map();
1415

@@ -43,5 +44,28 @@ module.exports = function(unpatched) {
4344
return ch;
4445
};
4546

47+
// tracingChannel(name) on affected versions where it's native (Node 16.17+/18.7+)
48+
// bypasses dc.channel above: native tracingChannel uses Node's internal channel()
49+
// registry directly, so its sub-channels (start/end/asyncStart/asyncEnd/error)
50+
// can still diverge from each other and from a bare dc.channel(name) lookup for
51+
// the same name. Construct the TracingChannel from the memoized dc.channel results
52+
// via the {start, end, ...} form so all paths see the same Channel identity.
53+
// (When the polyfill in patch-tracing-channel.js runs later — for !hasTracingChannel
54+
// versions — it overwrites this; that path already captures dc.channel transitively.)
55+
if (typeof dc_tracingChannel === 'function') {
56+
dc.tracingChannel = function tracingChannel(nameOrChannels) {
57+
if (typeof nameOrChannels !== 'string') {
58+
return dc_tracingChannel.call(this, nameOrChannels);
59+
}
60+
return dc_tracingChannel.call(this, {
61+
start: dc.channel(`tracing:${nameOrChannels}:start`),
62+
end: dc.channel(`tracing:${nameOrChannels}:end`),
63+
asyncStart: dc.channel(`tracing:${nameOrChannels}:asyncStart`),
64+
asyncEnd: dc.channel(`tracing:${nameOrChannels}:asyncEnd`),
65+
error: dc.channel(`tracing:${nameOrChannels}:error`),
66+
});
67+
};
68+
}
69+
4670
return dc;
4771
};

test/garbage-collection-patch.spec.js

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,75 @@ test('garbage-collection patch: distinct names get distinct Channel objects', t
6363
t.end();
6464
});
6565

66+
// Mock that also exposes a native-tracingChannel-like function. Native
67+
// tracingChannel(name) creates 5 sub-channels by calling the underlying
68+
// channel() function once for each sub-channel — so it inherits whatever
69+
// non-stability that channel() has. The patch needs to wrap tracingChannel
70+
// to feed memoized channels in instead.
71+
function mockUnpatchedWithTracingChannel() {
72+
const base = mockUnpatched();
73+
function tracingChannel(nameOrChannels) {
74+
if (typeof nameOrChannels === 'string') {
75+
// Mimic native: call channel() for each sub-channel name. With our
76+
// mock channel() returning fresh objects every call, sub-channels
77+
// diverge across two tracingChannel(name) calls without the patch.
78+
return {
79+
start: base.channel(`tracing:${nameOrChannels}:start`),
80+
end: base.channel(`tracing:${nameOrChannels}:end`),
81+
asyncStart: base.channel(`tracing:${nameOrChannels}:asyncStart`),
82+
asyncEnd: base.channel(`tracing:${nameOrChannels}:asyncEnd`),
83+
error: base.channel(`tracing:${nameOrChannels}:error`)
84+
};
85+
}
86+
return { ...nameOrChannels };
87+
}
88+
return Object.assign(base, { tracingChannel });
89+
}
90+
91+
test('garbage-collection patch: native tracingChannel sub-channels are stable per name', t => {
92+
const { channel, tracingChannel } = mockUnpatchedWithTracingChannel();
93+
const dc = patch({ channel, tracingChannel });
94+
95+
const a = dc.tracingChannel('foo');
96+
const b = dc.tracingChannel('foo');
97+
98+
t.strictEqual(a.start, b.start, 'tracingChannel(name).start stable across calls');
99+
t.strictEqual(a.end, b.end, 'tracingChannel(name).end stable across calls');
100+
t.strictEqual(a.asyncStart, b.asyncStart, 'tracingChannel(name).asyncStart stable across calls');
101+
t.strictEqual(a.asyncEnd, b.asyncEnd, 'tracingChannel(name).asyncEnd stable across calls');
102+
t.strictEqual(a.error, b.error, 'tracingChannel(name).error stable across calls');
103+
104+
t.end();
105+
});
106+
107+
test('garbage-collection patch: tracingChannel sub-channels match bare channel() lookups for the same name', t => {
108+
// The bug surface dd-trace's instrumentation hits: a publisher captures
109+
// tracingChannel(name).start in a closure, while a subscriber attaches
110+
// via dc.channel('tracing:name:start').subscribe(...). Without the patch,
111+
// those two see different Channel objects and never communicate.
112+
const { channel, tracingChannel } = mockUnpatchedWithTracingChannel();
113+
const dc = patch({ channel, tracingChannel });
114+
115+
const tc = dc.tracingChannel('observable');
116+
const bareStart = dc.channel('tracing:observable:start');
117+
118+
t.strictEqual(tc.start, bareStart, 'tracingChannel.start === channel("tracing:name:start")');
119+
t.end();
120+
});
121+
122+
test('garbage-collection patch: tracingChannel({channels}) form is passed through unchanged', t => {
123+
const { channel, tracingChannel } = mockUnpatchedWithTracingChannel();
124+
const dc = patch({ channel, tracingChannel });
125+
126+
const start = dc.channel('manual:start');
127+
const end = dc.channel('manual:end');
128+
const tc = dc.tracingChannel({ start, end });
129+
130+
t.strictEqual(tc.start, start, 'object-form start preserved');
131+
t.strictEqual(tc.end, end, 'object-form end preserved');
132+
t.end();
133+
});
134+
66135
test('garbage-collection patch: subscribers attach to the memoized Channel and receive publishes', t => {
67136
// This is the end-to-end shape the bug produces: the publisher captures
68137
// a Channel from one call, the subscriber attaches via a later call.

0 commit comments

Comments
 (0)