diff --git a/checks.js b/checks.js index 54c14e6..0f24802 100644 --- a/checks.js +++ b/checks.js @@ -56,9 +56,9 @@ function hasSyncUnsubscribeBug() { } module.exports.hasSyncUnsubscribeBug = hasSyncUnsubscribeBug; -// if there is a TracingChannel#hasSubscribers() getter +// if there is a TracingChannel#hasSubscribers() getter and the trace*() +// early-exit when no subscribers — both shipped in the same Node PR. // @see https://github.com/nodejs/node/pull/51915 -// TODO: note that we still need to add the TC early exit from this same version function hasTracingChannelHasSubscribers() { return MAJOR >= 22 || (MAJOR == 20 && MINOR >= 13); diff --git a/patch-tracing-channel-has-subscribers.js b/patch-tracing-channel-has-subscribers.js index c852996..eadc5b8 100644 --- a/patch-tracing-channel-has-subscribers.js +++ b/patch-tracing-channel-has-subscribers.js @@ -1,4 +1,6 @@ const { + ReflectApply, + FunctionPrototypeCallApply, ObjectDefineProperty, ObjectGetPrototypeOf, } = require('./primordials.js'); @@ -13,15 +15,87 @@ module.exports = function (unpatched) { ObjectDefineProperty(protoTrCh, 'hasSubscribers', { get: function () { - return this.start.hasSubscribers - || this.end.hasSubscribers - || this.asyncStart.hasSubscribers - || this.asyncEnd.hasSubscribers - || this.error.hasSubscribers; + // Null-safe: this patch is also applied on top of the JS polyfill on + // older Node, where partial object-form TracingChannels are accepted. + const { start, end, asyncStart, asyncEnd, error } = this; + return (start && start.hasSubscribers) + || (end && end.hasSubscribers) + || (asyncStart && asyncStart.hasSubscribers) + || (asyncEnd && asyncEnd.hasSubscribers) + || (error && error.hasSubscribers); }, configurable: true }); + + // Match native Node.js >= 22 / >= 20.13 behavior: when no channel has + // subscribers, skip the entire tracing path and invoke fn directly. + // Per native semantics, this intentionally bypasses traceCallback's + // callback validation and tracePromise's thenable coercion. + // @see https://github.com/nodejs/node/pull/51915 + const origTraceSync = protoTrCh.traceSync; + if (typeof origTraceSync === 'function') { + protoTrCh.traceSync = function (fn, context, thisArg, a, b, c) { + const argc = arguments.length; + if (!this.hasSubscribers) { + if (argc <= 3) return FunctionPrototypeCallApply(fn, thisArg); + if (argc === 4) return FunctionPrototypeCallApply(fn, thisArg, a); + if (argc === 5) return FunctionPrototypeCallApply(fn, thisArg, a, b); + if (argc === 6) return FunctionPrototypeCallApply(fn, thisArg, a, b, c); + return ReflectApply(fn, thisArg, sliceFrom(arguments, 3)); + } + if (argc <= 3) return FunctionPrototypeCallApply(origTraceSync, this, fn, context, thisArg); + if (argc === 4) return FunctionPrototypeCallApply(origTraceSync, this, fn, context, thisArg, a); + if (argc === 5) return FunctionPrototypeCallApply(origTraceSync, this, fn, context, thisArg, a, b); + if (argc === 6) return FunctionPrototypeCallApply(origTraceSync, this, fn, context, thisArg, a, b, c); + return ReflectApply(origTraceSync, this, sliceFrom(arguments, 0)); + }; + } + + const origTracePromise = protoTrCh.tracePromise; + if (typeof origTracePromise === 'function') { + protoTrCh.tracePromise = function (fn, context, thisArg, a, b, c) { + const argc = arguments.length; + if (!this.hasSubscribers) { + if (argc <= 3) return FunctionPrototypeCallApply(fn, thisArg); + if (argc === 4) return FunctionPrototypeCallApply(fn, thisArg, a); + if (argc === 5) return FunctionPrototypeCallApply(fn, thisArg, a, b); + if (argc === 6) return FunctionPrototypeCallApply(fn, thisArg, a, b, c); + return ReflectApply(fn, thisArg, sliceFrom(arguments, 3)); + } + if (argc <= 3) return FunctionPrototypeCallApply(origTracePromise, this, fn, context, thisArg); + if (argc === 4) return FunctionPrototypeCallApply(origTracePromise, this, fn, context, thisArg, a); + if (argc === 5) return FunctionPrototypeCallApply(origTracePromise, this, fn, context, thisArg, a, b); + if (argc === 6) return FunctionPrototypeCallApply(origTracePromise, this, fn, context, thisArg, a, b, c); + return ReflectApply(origTracePromise, this, sliceFrom(arguments, 0)); + }; + } + + const origTraceCallback = protoTrCh.traceCallback; + if (typeof origTraceCallback === 'function') { + protoTrCh.traceCallback = function (fn, position, context, thisArg, a, b, c) { + const argc = arguments.length; + if (!this.hasSubscribers) { + if (argc <= 4) return FunctionPrototypeCallApply(fn, thisArg); + if (argc === 5) return FunctionPrototypeCallApply(fn, thisArg, a); + if (argc === 6) return FunctionPrototypeCallApply(fn, thisArg, a, b); + if (argc === 7) return FunctionPrototypeCallApply(fn, thisArg, a, b, c); + return ReflectApply(fn, thisArg, sliceFrom(arguments, 4)); + } + if (argc <= 4) return FunctionPrototypeCallApply(origTraceCallback, this, fn, position, context, thisArg); + if (argc === 5) return FunctionPrototypeCallApply(origTraceCallback, this, fn, position, context, thisArg, a); + if (argc === 6) return FunctionPrototypeCallApply(origTraceCallback, this, fn, position, context, thisArg, a, b); + if (argc === 7) return FunctionPrototypeCallApply(origTraceCallback, this, fn, position, context, thisArg, a, b, c); + return ReflectApply(origTraceCallback, this, sliceFrom(arguments, 0)); + }; + } } return dc; }; + +function sliceFrom(argsLike, from) { + const len = argsLike.length; + const out = new Array(len - from); + for (let i = from; i < len; i++) out[i - from] = argsLike[i]; + return out; +} diff --git a/patch-tracing-channel.js b/patch-tracing-channel.js index d3f957f..cf0a17e 100644 --- a/patch-tracing-channel.js +++ b/patch-tracing-channel.js @@ -73,8 +73,31 @@ module.exports = function (unpatched) { return done; } + get hasSubscribers() { + // Null-safe: partial object-form TracingChannels (e.g. {start, end, error}) + // are accepted by the constructor above, so missing async channels must + // not crash this probe. + const { start, end, asyncStart, asyncEnd, error } = this; + return (start && start.hasSubscribers) + || (end && end.hasSubscribers) + || (asyncStart && asyncStart.hasSubscribers) + || (asyncEnd && asyncEnd.hasSubscribers) + || (error && error.hasSubscribers); + } + + // Each trace* method opens with an inline early-exit matching native + // Node >= 22 / >= 20.13: when no channel has subscribers, skip tracing + // and invoke fn directly. Per native semantics, this intentionally + // bypasses traceCallback's callback validation and tracePromise's + // thenable coercion. @see https://github.com/nodejs/node/pull/51915 + // Each method only checks the channels it would publish to, preserving + // backward compat with partial object-form TracingChannels. + traceSync(fn, context = {}, thisArg, ...args) { const { start, end, error } = this; + if (!(start.hasSubscribers || end.hasSubscribers || error.hasSubscribers)) { + return ReflectApply(fn, thisArg, args); + } return start.runStores(context, () => { try { @@ -93,6 +116,13 @@ module.exports = function (unpatched) { tracePromise(fn, context = {}, thisArg, ...args) { const { start, end, asyncStart, asyncEnd, error } = this; + if (!(start.hasSubscribers + || end.hasSubscribers + || asyncStart.hasSubscribers + || asyncEnd.hasSubscribers + || error.hasSubscribers)) { + return ReflectApply(fn, thisArg, args); + } function reject(err) { context.error = err; @@ -131,6 +161,13 @@ module.exports = function (unpatched) { traceCallback(fn, position = -1, context = {}, thisArg, ...args) { const { start, end, asyncStart, asyncEnd, error } = this; + if (!(start.hasSubscribers + || end.hasSubscribers + || asyncStart.hasSubscribers + || asyncEnd.hasSubscribers + || error.hasSubscribers)) { + return ReflectApply(fn, thisArg, args); + } function wrappedCallback(err, res) { if (err) { diff --git a/primordials.js b/primordials.js index b00d084..a50d567 100644 --- a/primordials.js +++ b/primordials.js @@ -14,6 +14,9 @@ function arrayAtPolyfill(n) { } const ReflectApply = Reflect.apply; +// Tamper-resistant `fn.call(thisArg, ...rest)`. Survives userland setting +// `userFn.call = null` or `Function.prototype.call = null` after this load. +const FunctionPrototypeCallApply = Function.prototype.call.bind(Function.prototype.call); const PromiseReject = Promise.reject.bind(Promise); const PromiseResolve = Promise.resolve; const PromisePrototypeThen = makeCall(Promise.prototype.then); @@ -28,6 +31,7 @@ const SymbolFor = Symbol.for; module.exports = { ReflectApply, + FunctionPrototypeCallApply, PromiseReject, PromiseResolve, PromisePrototypeThen, diff --git a/test/test-diagnostics-channel-tracing-channel-callback-early-exit.spec.js b/test/test-diagnostics-channel-tracing-channel-callback-early-exit.spec.js new file mode 100644 index 0000000..ae2d179 --- /dev/null +++ b/test/test-diagnostics-channel-tracing-channel-callback-early-exit.spec.js @@ -0,0 +1,33 @@ +'use strict'; + +const test = require('tape'); +const common = require('./common.js'); +const dc = require('../dc-polyfill.js'); + +test('test-diagnostics-channel-tracing-channel-callback-early-exit', (t) => { + const channel = dc.tracingChannel('test-early-exit-callback'); + + const handlers = { + start: common.mustNotCall(), + end: common.mustNotCall(), + asyncStart: common.mustNotCall(), + asyncEnd: common.mustNotCall(), + error: common.mustNotCall() + }; + + const expected = { ok: true }; + + function fn(arg, cb) { + // Subscribe inside the traced fn — early exit has committed already, + // so no events should be published for this call. + channel.subscribe(handlers); + process.nextTick(cb, null, arg); + } + + channel.traceCallback(fn, -1, {}, null, expected, (err, value) => { + t.error(err); + t.strictEqual(value, expected); + channel.unsubscribe(handlers); + t.end(); + }); +}); diff --git a/test/test-diagnostics-channel-tracing-channel-partial-object.spec.js b/test/test-diagnostics-channel-tracing-channel-partial-object.spec.js new file mode 100644 index 0000000..241dac3 --- /dev/null +++ b/test/test-diagnostics-channel-tracing-channel-partial-object.spec.js @@ -0,0 +1,23 @@ +'use strict'; + +const test = require('tape'); +const checks = require('../checks.js'); +const dc = require('../dc-polyfill.js'); + +// Partial object-form TracingChannels are only accepted by the JS polyfill +// path (Node < 18.19). Native diagnostics_channel rejects them at construction. +if (checks.hasTracingChannel()) { + test.skip('partial object-form TC is only accepted by the polyfill', () => {}); +} else { + test('polyfill traceSync with partial {start, end, error} object works', (t) => { + const start = dc.channel('partial:start'); + const end = dc.channel('partial:end'); + const error = dc.channel('partial:error'); + const tc = dc.tracingChannel({ start, end, error }); + + t.doesNotThrow(() => tc.hasSubscribers, 'hasSubscribers must not throw'); + t.strictEqual(tc.hasSubscribers, false); + t.strictEqual(tc.traceSync(() => 7, {}, null), 7); + t.end(); + }); +} diff --git a/test/test-diagnostics-channel-tracing-channel-promise-early-exit.spec.js b/test/test-diagnostics-channel-tracing-channel-promise-early-exit.spec.js new file mode 100644 index 0000000..910eb16 --- /dev/null +++ b/test/test-diagnostics-channel-tracing-channel-promise-early-exit.spec.js @@ -0,0 +1,35 @@ +'use strict'; + +const test = require('tape'); +const common = require('./common.js'); +const dc = require('../dc-polyfill.js'); + +test('test-diagnostics-channel-tracing-channel-promise-early-exit', (t) => { + const channel = dc.tracingChannel('test-early-exit-promise'); + + const handlers = { + start: common.mustNotCall(), + end: common.mustNotCall(), + asyncStart: common.mustNotCall(), + asyncEnd: common.mustNotCall(), + error: common.mustNotCall() + }; + + const expected = { ok: true }; + + // Subscribe inside the traced fn — by then the early exit has already + // committed, so no events should be published for this call. + const result = channel.tracePromise(() => { + channel.subscribe(handlers); + return Promise.resolve(expected); + }, {}); + + result.then((value) => { + t.strictEqual(value, expected); + channel.unsubscribe(handlers); + t.end(); + }, (err) => { + t.fail(err); + t.end(); + }); +}); diff --git a/test/test-diagnostics-channel-tracing-channel-shadowed-call.spec.js b/test/test-diagnostics-channel-tracing-channel-shadowed-call.spec.js new file mode 100644 index 0000000..69c9f30 --- /dev/null +++ b/test/test-diagnostics-channel-tracing-channel-shadowed-call.spec.js @@ -0,0 +1,33 @@ +'use strict'; + +const test = require('tape'); +const dc = require('../dc-polyfill.js'); + +test('traceSync survives shadowed fn.call when no subscribers', (t) => { + const channel = dc.tracingChannel('test-shadowed-call-sync'); + const fn = function () { return 42; }; + fn.call = null; + t.strictEqual(channel.traceSync(fn, {}, null), 42); + t.end(); +}); + +test('tracePromise survives shadowed fn.call when no subscribers', (t) => { + const channel = dc.tracingChannel('test-shadowed-call-promise'); + const expected = Promise.resolve(42); + const fn = function () { return expected; }; + fn.call = null; + const result = channel.tracePromise(fn, {}, null); + t.strictEqual(result, expected); + t.end(); +}); + +test('traceCallback survives shadowed fn.call when no subscribers', (t) => { + const channel = dc.tracingChannel('test-shadowed-call-callback'); + const fn = function (cb) { cb(null, 42); }; + fn.call = null; + channel.traceCallback(fn, -1, {}, null, (err, value) => { + t.error(err); + t.strictEqual(value, 42); + t.end(); + }); +}); diff --git a/test/test-diagnostics-channel-tracing-channel-sync-early-exit.js b/test/test-diagnostics-channel-tracing-channel-sync-early-exit.spec.js similarity index 88% rename from test/test-diagnostics-channel-tracing-channel-sync-early-exit.js rename to test/test-diagnostics-channel-tracing-channel-sync-early-exit.spec.js index da15632..413c0a3 100644 --- a/test/test-diagnostics-channel-tracing-channel-sync-early-exit.js +++ b/test/test-diagnostics-channel-tracing-channel-sync-early-exit.spec.js @@ -5,9 +5,7 @@ const common = require('./common.js'); const dc = require('../dc-polyfill.js'); test('test-diagnostics-channel-tracing-channel-sync-early-exit', (t) => { - t.plan(0); - - const channel = dc.tracingChannel('test'); + const channel = dc.tracingChannel('test-early-exit-sync'); const handlers = { start: common.mustNotCall(), @@ -22,4 +20,6 @@ test('test-diagnostics-channel-tracing-channel-sync-early-exit', (t) => { channel.traceSync(() => { channel.subscribe(handlers); }, {}); + + t.end(); });