Skip to content

Commit 4e60afb

Browse files
committed
diagnostics_channel: fix USDT hot path under startup snapshot
The publish() hot path captured the probe semaphore Uint16Array view at module load. This module is baked into the startup snapshot, and the view's native backing store cannot be serialized: the captured view is detached when the snapshot is deserialized, so publish() never called emitPublishProbe() in snapshot builds and the dc__publish probe never fired for JavaScript publishes even with a tracer attached. Caught by the new bpftrace CI job. Resolve the view lazily from the binding (once per process; 'null' marks a USDT-less build) and add positive-path coverage to test-diagnostics-channel-usdt.js: force the semaphore and verify that publish() calls emitPublishProbe() for string-named channels, never for symbol-named ones, and not when the semaphore is 0.
1 parent 574e3d7 commit 4e60afb

2 files changed

Lines changed: 86 additions & 17 deletions

File tree

lib/diagnostics_channel.js

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,15 @@ const {
3232
const { triggerUncaughtException } = internalBinding('errors');
3333

3434
const dc_binding = internalBinding('diagnostics_channel');
35-
const { subscribers: subscriberCounts, probeSemaphore } = dc_binding;
36-
// When compiled without USDT support, probeSemaphore is undefined and
37-
// emitPublishProbe does not exist. Capture this once at load time so that
38-
// when USDT is absent, the hot path in publish() can skip all probe logic
39-
// with a single boolean check.
40-
const hasUSDT = probeSemaphore !== undefined;
35+
const { subscribers: subscriberCounts } = dc_binding;
36+
// The USDT probe semaphore is exposed by the binding as a Uint16Array
37+
// view over static native memory. It must be resolved lazily rather than
38+
// captured at module load: this module is included in the startup
39+
// snapshot, and the view's native backing store cannot be serialized:
40+
// a view captured while building the snapshot is detached when the
41+
// snapshot is deserialized. `null` marks a USDT-less build after the
42+
// first resolution so that the hot path stays branch-only in that case.
43+
let probeSemaphore;
4144

4245
const { WeakReference } = require('internal/util');
4346

@@ -163,7 +166,12 @@ class ActiveChannel {
163166
}
164167

165168
publish(data) {
166-
if (hasUSDT && probeSemaphore[0] > 0 && typeof this.name === 'string') {
169+
if (probeSemaphore === undefined) {
170+
probeSemaphore = dc_binding.probeSemaphore ?? null;
171+
}
172+
if (probeSemaphore !== null &&
173+
probeSemaphore[0] > 0 &&
174+
typeof this.name === 'string') {
167175
dc_binding.emitPublishProbe(this.name, data);
168176
}
169177
const subscribers = this._subscribers;

test/parallel/test-diagnostics-channel-usdt.js

Lines changed: 71 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ const binding = internalBinding('diagnostics_channel');
2323
);
2424

2525
if (probeSemaphore !== undefined) {
26-
// Without a tracer attached the semaphore must be 0 (Linux SystemTap
27-
// dtrace -h path) or 1 (macOS/FreeBSD/illumos dtrace -h path, or
28-
// fallback sys/sdt.h path where the probe always fires).
26+
// Without a tracer attached the semaphore must be 0 (Linux, committed
27+
// SystemTap-generated header) or 1 (macOS --with-dtrace path, which
28+
// has no native semaphore).
2929
assert.ok(
3030
probeSemaphore[0] === 0 || probeSemaphore[0] === 1,
3131
`Expected semaphore to be 0 or 1, got ${probeSemaphore[0]}`,
@@ -41,10 +41,11 @@ const binding = internalBinding('diagnostics_channel');
4141

4242
// --- JS probe guard: verify emitPublishProbe is called/skipped ---
4343

44-
// When the semaphore is > 0 (Tier 2 fallback), emitPublishProbe must be
45-
// called for string-named channels and must NOT be called for symbol-named
46-
// channels. When the semaphore is 0 (Tier 1, no tracer) or USDT is absent,
47-
// emitPublishProbe must never be called.
44+
// When the semaphore is > 0 (macOS --with-dtrace, which has no native
45+
// semaphore), emitPublishProbe must be called for string-named channels and
46+
// must NOT be called for symbol-named channels. When the semaphore is 0
47+
// (Linux Tier 1, no tracer) or USDT is absent, emitPublishProbe must never
48+
// be called.
4849
{
4950
const { probeSemaphore, emitPublishProbe } = binding;
5051
const semaphoreEnabled = probeSemaphore !== undefined &&
@@ -68,10 +69,12 @@ const binding = internalBinding('diagnostics_channel');
6869

6970
if (semaphoreEnabled) {
7071
assert.strictEqual(probeCallCount, 1,
71-
'emitPublishProbe should be called once for string-named channel');
72+
`emitPublishProbe should be called once for ` +
73+
`string-named channel, got ${probeCallCount}`);
7274
} else {
7375
assert.strictEqual(probeCallCount, 0,
74-
'emitPublishProbe should not be called when semaphore is 0');
76+
`emitPublishProbe should not be called when the ` +
77+
`semaphore is 0, got ${probeCallCount}`);
7578
}
7679

7780
// Symbol-named channel — probe must never fire regardless of semaphore.
@@ -84,14 +87,72 @@ const binding = internalBinding('diagnostics_channel');
8487
symCh.unsubscribe(symSub);
8588

8689
assert.strictEqual(probeCallCount, 0,
87-
'emitPublishProbe must not be called for symbol-named channels');
90+
`emitPublishProbe must not be called for symbol-named ` +
91+
`channels, got ${probeCallCount}`);
8892

8993
// Restore original.
9094
if (origProbe !== undefined) {
9195
binding.emitPublishProbe = origProbe;
9296
}
9397
}
9498

99+
// --- JS probe guard: positive path through the public publish() API ---
100+
101+
// Force the semaphore to look "attached" and verify that the hot path in
102+
// lib/internal/diagnostics_channel actually calls emitPublishProbe. This
103+
// exercises the module-level view of the semaphore (which must survive
104+
// startup-snapshot deserialization) rather than the binding directly.
105+
if (binding.probeSemaphore !== undefined) {
106+
const { probeSemaphore } = binding;
107+
const origSemaphore = probeSemaphore[0];
108+
let probeCalls = [];
109+
const origProbe = binding.emitPublishProbe;
110+
binding.emitPublishProbe = (name) => probeCalls.push(name);
111+
112+
try {
113+
probeSemaphore[0] = 1;
114+
const ch = dc.channel('test:usdt:probe-positive');
115+
const subscriber = common.mustCall();
116+
ch.subscribe(subscriber);
117+
probeCalls = [];
118+
ch.publish({ probePositive: true });
119+
ch.unsubscribe(subscriber);
120+
121+
assert.deepStrictEqual(probeCalls, ['test:usdt:probe-positive'],
122+
`publish() must call emitPublishProbe for string-named channels ` +
123+
`when the semaphore is > 0, got ${JSON.stringify(probeCalls)}`);
124+
125+
// Symbol-named channels must never emit the probe.
126+
probeCalls = [];
127+
const sym = Symbol('test:usdt:symbol-probe-positive');
128+
const symCh = dc.channel(sym);
129+
const symSub = common.mustCall();
130+
symCh.subscribe(symSub);
131+
symCh.publish({ symbolPositive: true });
132+
symCh.unsubscribe(symSub);
133+
134+
assert.deepStrictEqual(probeCalls, [],
135+
`publish() must not call emitPublishProbe for symbol-named ` +
136+
`channels, got ${JSON.stringify(probeCalls)}`);
137+
138+
// With the semaphore back at 0, the probe must not be emitted.
139+
probeSemaphore[0] = 0;
140+
probeCalls = [];
141+
const ch0 = dc.channel('test:usdt:probe-negative');
142+
const sub0 = common.mustCall();
143+
ch0.subscribe(sub0);
144+
ch0.publish({ probeNegative: true });
145+
ch0.unsubscribe(sub0);
146+
147+
assert.deepStrictEqual(probeCalls, [],
148+
`publish() must not call emitPublishProbe when the semaphore is 0, ` +
149+
`got ${JSON.stringify(probeCalls)}`);
150+
} finally {
151+
probeSemaphore[0] = origSemaphore;
152+
binding.emitPublishProbe = origProbe;
153+
}
154+
}
155+
95156
// --- Publish with and without subscribers ---
96157

97158
// String-named channel with subscribers.

0 commit comments

Comments
 (0)