diff --git a/src/js/node/inspector.ts b/src/js/node/inspector.ts index c03dd8c27336..355e4cb11976 100644 --- a/src/js/node/inspector.ts +++ b/src/js/node/inspector.ts @@ -16,9 +16,11 @@ const DateNow = Date.now; // errors as plain objects, not Error instances). const kProtocolError = Symbol("kProtocolError"); -// Native profiler functions exposed via $newCppFunction +// Native profiler functions exposed via $newCppFunction. +// startCPUProfiler() returns a per-call owner token; stopCPUProfiler(owner) +// returns the JSON profile covering that owner's window. const startCPUProfiler = $newCppFunction("JSInspectorProfiler.cpp", "jsFunction_startCPUProfiler", 0); -const stopCPUProfiler = $newCppFunction("JSInspectorProfiler.cpp", "jsFunction_stopCPUProfiler", 0); +const stopCPUProfiler = $newCppFunction("JSInspectorProfiler.cpp", "jsFunction_stopCPUProfiler", 1); const setCPUSamplingInterval = $newCppFunction("JSInspectorProfiler.cpp", "jsFunction_setCPUSamplingInterval", 1); const isCPUProfilerRunning = $newCppFunction("JSInspectorProfiler.cpp", "jsFunction_isCPUProfilerRunning", 0); const startPreciseCoverage = $newCppFunction("JSInspectorProfiler.cpp", "jsFunction_startPreciseCoverage", 0); @@ -415,6 +417,11 @@ function collectCoverageScripts(): any[] | Error { class Session extends EventEmitter { #connected = false; #profilerEnabled = false; + // Nonzero while this session holds a live CPU-profiler owner. The underlying + // JSC SamplingProfiler is shared VM-wide (CLI `--cpu-prof` and other + // Sessions may own it concurrently), so Profiler.stop/disable/disconnect + // must release only this session's claim. + #cpuProfilerOwner = 0; #preciseCoverageEnabled = false; #preciseCoverageCallCount = false; #preciseCoverageDetailed = false; @@ -439,7 +446,10 @@ class Session extends EventEmitter { disconnect() { if (!this.#connected) return; - if (isCPUProfilerRunning()) stopCPUProfiler(); + if (this.#cpuProfilerOwner !== 0) { + stopCPUProfiler(this.#cpuProfilerOwner); + this.#cpuProfilerOwner = 0; + } if (this.#preciseCoverageEnabled) { stopPreciseCoverage(); this.#preciseCoverageEnabled = false; @@ -497,15 +507,13 @@ class Session extends EventEmitter { } }); } else { - // Sync throw for errors when no callback - if (result instanceof Error) { - throw result; - } - if (result !== null && typeof result === "object" && kProtocolError in result) { - const protocolError = result[kProtocolError]; - const error = new Error(protocolError.message); - error.code = protocolError.code; - throw error; + // Node's post() without a callback is fire-and-forget: an error + // response to the dispatched command is dropped, never thrown. The + // synchronous throws above (argument validation, not-connected) run + // before dispatch and match Node. Returning the success result is a + // Bun convenience; Node returns undefined. + if (result instanceof Error || (result !== null && typeof result === "object" && kProtocolError in result)) { + return undefined; } return result; } @@ -528,8 +536,9 @@ class Session extends EventEmitter { return {}; case "Profiler.disable": - if (isCPUProfilerRunning()) { - stopCPUProfiler(); + if (this.#cpuProfilerOwner !== 0) { + stopCPUProfiler(this.#cpuProfilerOwner); + this.#cpuProfilerOwner = 0; } // V8's Profiler agent stops precise coverage on disable; without this // the control-flow profiler keeps instrumenting newly-compiled code. @@ -542,16 +551,19 @@ class Session extends EventEmitter { case "Profiler.start": if (!this.#profilerEnabled) return $ERR_INSPECTOR_COMMAND("-32000: Profiler is not enabled"); - if (!isCPUProfilerRunning()) startCPUProfiler(); + if (this.#cpuProfilerOwner === 0) this.#cpuProfilerOwner = startCPUProfiler(); return {}; - case "Profiler.stop": - if (!isCPUProfilerRunning()) return $ERR_INSPECTOR_COMMAND("-32000: Profiler is not started"); + case "Profiler.stop": { + if (this.#cpuProfilerOwner === 0) return $ERR_INSPECTOR_COMMAND("-32000: Profiler is not started"); + const owner = this.#cpuProfilerOwner; + this.#cpuProfilerOwner = 0; try { - return { profile: JSON.parse(stopCPUProfiler()) }; + return { profile: JSON.parse(stopCPUProfiler(owner)) }; } catch (e) { return $ERR_INSPECTOR_COMMAND(`-32000: Failed to parse profile JSON: ${e}`); } + } case "Profiler.setSamplingInterval": { if (isCPUProfilerRunning()) diff --git a/src/jsc/bindings/BunCPUProfiler.cpp b/src/jsc/bindings/BunCPUProfiler.cpp index 978ab5660494..5212cb528f23 100644 --- a/src/jsc/bindings/BunCPUProfiler.cpp +++ b/src/jsc/bindings/BunCPUProfiler.cpp @@ -29,11 +29,33 @@ void Bun__setSamplingInterval(int intervalMicroseconds) namespace Bun { -// Store the profiling start time in microseconds since Unix epoch -static thread_local double s_profilingStartTime = 0.0; // Set sampling interval to 1ms (1000 microseconds) to match Node.js static thread_local int s_samplingInterval = 1000; -static thread_local bool s_isProfilerRunning = false; + +// A single JSC::SamplingProfiler exists per VM, but multiple independent +// profile windows can be open at once (e.g. `--cpu-prof` at the CLI plus one +// or more `node:inspector` Sessions). Each caller acquires an owner, which +// records its own start timestamp. The underlying sampler only runs while at +// least one owner is live, and stopping one owner snapshots the shared trace +// buffer filtered to that owner's window without disturbing the others. +struct ProfilerOwner { + uint32_t id; + // Wall-clock start in microseconds since epoch, used as profile.startTime. + double startTimeMicros; + // Monotonic timestamp at acquisition, used to filter traces. + MonotonicTime startTimestamp; +}; + +static thread_local WTF::Vector s_profilerOwners; +static thread_local uint32_t s_nextProfilerOwnerId = 1; +// Owner held on behalf of the legacy void-returning start/stop entry points +// (CLI `--cpu-prof` and per-Worker profiling). 0 means not held. +static thread_local uint32_t s_implicitProfilerOwner = 0; +// Traces drained from the SamplingProfiler while more than one owner is live. +// The referenced executables/callees stay alive via the profiler's +// m_liveCellPointers (visited during GC) until clearData() runs when the last +// owner releases. +static thread_local WTF::Vector s_retainedTraces; void setSamplingInterval(int intervalMicroseconds) { @@ -42,24 +64,32 @@ void setSamplingInterval(int intervalMicroseconds) bool isCPUProfilerRunning() { - return s_isProfilerRunning; + return !s_profilerOwners.isEmpty(); +} + +uint32_t acquireCPUProfilerOwner(JSC::VM& vm) +{ + uint32_t id = s_nextProfilerOwnerId++; + MonotonicTime now = MonotonicTime::now(); + double startTimeMicros = now.approximate().secondsSinceEpoch().value() * 1000000.0; + bool firstOwner = s_profilerOwners.isEmpty(); + s_profilerOwners.append({ id, startTimeMicros, now }); + + if (firstOwner) { + auto stopwatch = WTF::Stopwatch::create(); + stopwatch->start(); + JSC::SamplingProfiler& samplingProfiler = vm.ensureSamplingProfiler(WTF::move(stopwatch)); + samplingProfiler.setTimingInterval(WTF::Seconds::fromMicroseconds(s_samplingInterval)); + samplingProfiler.noticeCurrentThreadAsJSCExecutionThread(); + samplingProfiler.start(); + } + return id; } void startCPUProfiler(JSC::VM& vm) { - // Capture the wall clock time when profiling starts (before creating stopwatch) - // This will be used as the profile's startTime - s_profilingStartTime = MonotonicTime::now().approximate().secondsSinceEpoch().value() * 1000000.0; - - // Create a stopwatch and start it - auto stopwatch = WTF::Stopwatch::create(); - stopwatch->start(); - - JSC::SamplingProfiler& samplingProfiler = vm.ensureSamplingProfiler(WTF::move(stopwatch)); - samplingProfiler.setTimingInterval(WTF::Seconds::fromMicroseconds(s_samplingInterval)); - samplingProfiler.noticeCurrentThreadAsJSCExecutionThread(); - samplingProfiler.start(); - s_isProfilerRunning = true; + if (s_implicitProfilerOwner == 0) + s_implicitProfilerOwner = acquireCPUProfilerOwner(vm); } struct ProfileNode { @@ -270,13 +300,11 @@ static WTF::String formatCodeSpan(const WTF::String& str) } // Helper to generate a minimal valid cpuprofile JSON with no samples -static WTF::String generateEmptyProfileJSON() +static WTF::String generateEmptyProfileJSON(double startTimeMicros) { - // Return a minimal valid Chrome DevTools CPU profile format - // Use s_profilingStartTime if available, otherwise fall back to current time long long timestamp; - if (s_profilingStartTime > 0) - timestamp = static_cast(s_profilingStartTime); + if (startTimeMicros > 0) + timestamp = static_cast(startTimeMicros); else timestamp = static_cast(WTF::WallTime::now().secondsSinceEpoch().value() * 1000000.0); @@ -289,53 +317,34 @@ static WTF::String generateEmptyProfileJSON() return sb.toString(); } -// Unified function that stops the profiler and generates requested output formats -void stopCPUProfiler(JSC::VM& vm, WTF::String* outJSON, WTF::String* outText) +// Builds the Chrome DevTools JSON and/or Markdown text report from the +// already-drained traces, including only those with timestamp >= minTimestamp +// and using startTimeMicros as the profile's startTime baseline. Must be +// called under JSLockHolder + DeferGC; frames reference live GC cells. +static void buildProfileOutput(JSC::VM& vm, WTF::Vector& stackTraces, + MonotonicTime minTimestamp, double startTimeMicros, WTF::String* outJSON, WTF::String* outText) { - s_isProfilerRunning = false; - - JSC::SamplingProfiler* profiler = vm.samplingProfiler(); - if (!profiler) { - if (outJSON) *outJSON = WTF::String(); - if (outText) *outText = WTF::String(); - return; - } - - // JSLock is re-entrant, so always acquiring it handles both JS and shutdown contexts - JSC::JSLockHolder locker(vm); - - // Defer GC while we're working with stack traces - JSC::DeferGC deferGC(vm); - - // Pause the profiler while holding the lock - auto& lock = profiler->getLock(); - WTF::Locker profilerLocker { lock }; - profiler->pause(); - - // releaseStackTraces() calls processUnverifiedStackTraces() internally - auto stackTraces = profiler->releaseStackTraces(); - profiler->clearData(); - - // If neither output is requested, we're done if (!outJSON && !outText) return; - if (stackTraces.isEmpty()) { - if (outJSON) *outJSON = generateEmptyProfileJSON(); - if (outText) *outText = "No samples collected.\n"_s; - return; - } - - // Sort traces by timestamp once for both formats + // Sort traces by timestamp once for both formats, including only those in + // this owner's window. WTF::Vector sortedIndices; sortedIndices.reserveInitialCapacity(stackTraces.size()); for (size_t i = 0; i < stackTraces.size(); i++) { - sortedIndices.append(i); + if (stackTraces[i].timestamp >= minTimestamp) + sortedIndices.append(i); } std::sort(sortedIndices.begin(), sortedIndices.end(), [&stackTraces](size_t a, size_t b) { return stackTraces[a].timestamp < stackTraces[b].timestamp; }); + if (sortedIndices.isEmpty()) { + if (outJSON) *outJSON = generateEmptyProfileJSON(startTimeMicros); + if (outText) *outText = "No samples collected.\n"_s; + return; + } + // Generate JSON format if requested if (outJSON) { // Map from stack frame signature to node ID @@ -357,8 +366,8 @@ void stopCPUProfiler(JSC::VM& vm, WTF::String* outJSON, WTF::String* outText) WTF::Vector samples; WTF::Vector timeDeltas; - double startTime = s_profilingStartTime; - double lastTime = s_profilingStartTime; + double startTime = startTimeMicros; + double lastTime = startTimeMicros; for (size_t idx : sortedIndices) { auto& stackTrace = stackTraces[idx]; @@ -617,14 +626,14 @@ void stopCPUProfiler(JSC::VM& vm, WTF::String* outJSON, WTF::String* outText) // Generate text format if requested if (outText) { - double startTime = s_profilingStartTime; - double lastTime = s_profilingStartTime; + double startTime = startTimeMicros; + double lastTime = startTimeMicros; double endTime = startTime; WTF::HashMap functionStatsMap; long long totalTimeUs = 0; - int totalSamples = static_cast(stackTraces.size()); + int totalSamples = static_cast(sortedIndices.size()); for (size_t idx : sortedIndices) { auto& stackTrace = stackTraces[idx]; @@ -928,6 +937,85 @@ void stopCPUProfiler(JSC::VM& vm, WTF::String* outJSON, WTF::String* outText) } } +void releaseCPUProfilerOwner(JSC::VM& vm, uint32_t ownerId, WTF::String* outJSON, WTF::String* outText) +{ + size_t ownerIndex = WTF::notFound; + for (size_t i = 0; i < s_profilerOwners.size(); i++) { + if (s_profilerOwners[i].id == ownerId) { + ownerIndex = i; + break; + } + } + if (ownerIndex == WTF::notFound) { + if (outJSON) *outJSON = WTF::String(); + if (outText) *outText = WTF::String(); + return; + } + ProfilerOwner owner = s_profilerOwners[ownerIndex]; + s_profilerOwners.removeAt(ownerIndex); + bool lastOwner = s_profilerOwners.isEmpty(); + + JSC::SamplingProfiler* profiler = vm.samplingProfiler(); + if (!profiler) { + if (outJSON) *outJSON = generateEmptyProfileJSON(owner.startTimeMicros); + if (outText) *outText = "No samples collected.\n"_s; + s_retainedTraces.clear(); + return; + } + + // JSLock is re-entrant, so always acquiring it handles both JS and shutdown contexts + JSC::JSLockHolder locker(vm); + JSC::DeferGC deferGC(vm); + + auto& lock = profiler->getLock(); + WTF::Locker profilerLocker { lock }; + + // Drain newly-collected traces into the retained buffer. releaseStackTraces() + // moves m_stackTraces out but leaves m_liveCellPointers intact, so the + // executables/callees referenced by s_retainedTraces stay GC-live until the + // final owner's clearData() below. + { + auto newTraces = profiler->releaseStackTraces(); + s_retainedTraces.reserveCapacity(s_retainedTraces.size() + newTraces.size()); + for (auto& trace : newTraces) + s_retainedTraces.append(WTF::move(trace)); + } + + buildProfileOutput(vm, s_retainedTraces, owner.startTimestamp, owner.startTimeMicros, outJSON, outText); + + if (lastOwner) { + profiler->pause(); + profiler->clearData(); + s_retainedTraces.clear(); + } else { + // Bound memory: drop traces older than every remaining owner's window. + // StackTrace is move-constructible but not move-assignable, so rebuild + // rather than compact in place. + MonotonicTime minStart = s_profilerOwners[0].startTimestamp; + for (size_t i = 1; i < s_profilerOwners.size(); i++) + minStart = std::min(minStart, s_profilerOwners[i].startTimestamp); + WTF::Vector survivors; + survivors.reserveInitialCapacity(s_retainedTraces.size()); + for (auto& t : s_retainedTraces) { + if (t.timestamp >= minStart) + survivors.append(WTF::move(t)); + } + s_retainedTraces = WTF::move(survivors); + } +} + +void stopCPUProfiler(JSC::VM& vm, WTF::String* outJSON, WTF::String* outText) +{ + if (s_implicitProfilerOwner == 0) { + if (outJSON) *outJSON = WTF::String(); + if (outText) *outText = WTF::String(); + return; + } + uint32_t owner = s_implicitProfilerOwner; + s_implicitProfilerOwner = 0; + releaseCPUProfilerOwner(vm, owner, outJSON, outText); +} + } // namespace Bun extern "C" void Bun__startCPUProfiler(JSC::VM* vm) diff --git a/src/jsc/bindings/BunCPUProfiler.h b/src/jsc/bindings/BunCPUProfiler.h index e978360b2912..e263a6e3f72f 100644 --- a/src/jsc/bindings/BunCPUProfiler.h +++ b/src/jsc/bindings/BunCPUProfiler.h @@ -13,11 +13,19 @@ namespace Bun { void setSamplingInterval(int intervalMicroseconds); bool isCPUProfilerRunning(); -// Start the CPU profiler -void startCPUProfiler(JSC::VM& vm); +// Register a new profiler owner and start the VM's SamplingProfiler if this +// is the first one. Returns a nonzero owner token for releaseCPUProfilerOwner. +uint32_t acquireCPUProfilerOwner(JSC::VM& vm); + +// Release an owner returned by acquireCPUProfilerOwner(). Writes the profile +// (covering only this owner's window) to the requested non-null out-params, +// and stops the underlying SamplingProfiler once the last owner releases. +void releaseCPUProfilerOwner(JSC::VM& vm, uint32_t owner, WTF::String* outJSON, WTF::String* outText); -// Stop the CPU profiler and get profile data in requested formats. -// Pass non-null pointers for the formats you want. Null pointers are skipped. +// Single-implicit-owner convenience wrappers (CLI `--cpu-prof`, per-Worker +// profiling). startCPUProfiler is a no-op if the implicit owner is already +// held; stopCPUProfiler releases it. +void startCPUProfiler(JSC::VM& vm); void stopCPUProfiler(JSC::VM& vm, WTF::String* outJSON, WTF::String* outText); } // namespace Bun diff --git a/src/jsc/bindings/JSInspectorProfiler.cpp b/src/jsc/bindings/JSInspectorProfiler.cpp index a23358bf56c2..481298576bef 100644 --- a/src/jsc/bindings/JSInspectorProfiler.cpp +++ b/src/jsc/bindings/JSInspectorProfiler.cpp @@ -19,16 +19,17 @@ using namespace JSC; JSC_DECLARE_HOST_FUNCTION(jsFunction_startCPUProfiler); JSC_DEFINE_HOST_FUNCTION(jsFunction_startCPUProfiler, (JSGlobalObject * globalObject, CallFrame*)) { - Bun::startCPUProfiler(globalObject->vm()); - return JSValue::encode(jsUndefined()); + uint32_t owner = Bun::acquireCPUProfilerOwner(globalObject->vm()); + return JSValue::encode(jsNumber(owner)); } JSC_DECLARE_HOST_FUNCTION(jsFunction_stopCPUProfiler); -JSC_DEFINE_HOST_FUNCTION(jsFunction_stopCPUProfiler, (JSGlobalObject * globalObject, CallFrame*)) +JSC_DEFINE_HOST_FUNCTION(jsFunction_stopCPUProfiler, (JSGlobalObject * globalObject, CallFrame* callFrame)) { auto& vm = globalObject->vm(); + uint32_t owner = callFrame->argument(0).toUInt32(globalObject); WTF::String result; - Bun::stopCPUProfiler(vm, &result, nullptr); + Bun::releaseCPUProfilerOwner(vm, owner, &result, nullptr); return JSValue::encode(jsString(vm, result)); } diff --git a/test/js/node/inspector/inspector-profiler.test.ts b/test/js/node/inspector/inspector-profiler.test.ts index 30518c0178ee..d2b237b7840b 100644 --- a/test/js/node/inspector/inspector-profiler.test.ts +++ b/test/js/node/inspector/inspector-profiler.test.ts @@ -1,8 +1,16 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { bunEnv, bunExe, tempDir } from "harness"; +import { readdirSync, readFileSync } from "node:fs"; import inspector from "node:inspector"; import inspectorPromises from "node:inspector/promises"; +// Node's Session.post() without a callback is fire-and-forget (command errors +// are dropped), so tests that assert on a command error must go through the +// callback path. +function postErr(session: inspector.Session, method: string, params?: object): Promise { + return new Promise(resolve => session.post(method, params as any, err => resolve(err))); +} + // Mirrors how vitest's @vitest/coverage-v8 provider drives the inspector: a // promise Session, Profiler.enable, startPreciseCoverage, evaluating modules // through node:vm, then takePreciseCoverage. @@ -209,8 +217,10 @@ describe("node:inspector", () => { expect(result).toEqual({}); }); - test("Profiler.start without enable throws", () => { - expect(() => session.post("Profiler.start")).toThrow("not enabled"); + test("Profiler.start without enable reports an error", async () => { + const err = await postErr(session, "Profiler.start"); + expect(err).toBeInstanceOf(Error); + expect(err.message).toContain("not enabled"); }); test("Profiler.start after enable succeeds", () => { @@ -219,9 +229,11 @@ describe("node:inspector", () => { expect(result).toEqual({}); }); - test("Profiler.stop without start throws", () => { + test("Profiler.stop without start reports an error", async () => { session.post("Profiler.enable"); - expect(() => session.post("Profiler.stop")).toThrow("not started"); + const err = await postErr(session, "Profiler.stop"); + expect(err).toBeInstanceOf(Error); + expect(err.message).toContain("not started"); }); test("Profiler.stop returns valid profile", () => { @@ -326,19 +338,19 @@ describe("node:inspector", () => { expect(result).toEqual({}); }); - test("Profiler.setSamplingInterval throws if profiler is running", () => { + test("Profiler.setSamplingInterval reports an error if profiler is running", async () => { session.post("Profiler.enable"); session.post("Profiler.start"); - expect(() => session.post("Profiler.setSamplingInterval", { interval: 500 })).toThrow( - "Cannot change sampling interval while profiler is running", - ); + const err = await postErr(session, "Profiler.setSamplingInterval", { interval: 500 }); + expect(err).toBeInstanceOf(Error); + expect(err.message).toContain("Cannot change sampling interval while profiler is running"); session.post("Profiler.stop"); }); - test("Profiler.setSamplingInterval requires positive interval", () => { + test("Profiler.setSamplingInterval requires positive interval", async () => { session.post("Profiler.enable"); - expect(() => session.post("Profiler.setSamplingInterval", { interval: 0 })).toThrow(); - expect(() => session.post("Profiler.setSamplingInterval", { interval: -1 })).toThrow(); + expect(await postErr(session, "Profiler.setSamplingInterval", { interval: 0 })).toBeInstanceOf(Error); + expect(await postErr(session, "Profiler.setSamplingInterval", { interval: -1 })).toBeInstanceOf(Error); }); test("double Profiler.start is a no-op", () => { @@ -386,6 +398,138 @@ describe("node:inspector", () => { }); }); + // A single JSC SamplingProfiler backs every profiler owner on a VM, so each + // inspector Session and the CLI `--cpu-prof` flag must hold independent + // claims on it. These run in subprocesses because they assert on the + // VM-global profiler state and do real CPU-bound work. + describe("profiler ownership", () => { + const work = + "const work = ms => { const t = performance.now(); let x = 0; while (performance.now() - t < ms) x += Math.sqrt(x + 1); };"; + const postFn = + "const post = (s, m, p) => new Promise((res, rej) => s.post(m, p, (e, r) => (e ? rej(e) : res(r))));"; + + test.concurrent("a bare Session connect/disconnect does not clear --cpu-prof's samples", async () => { + using dir = tempDir("inspector-cpuprof-owner", { + "child.mjs": ` + import inspector from "node:inspector"; + ${work} + work(200); + const s = new inspector.Session(); + s.connect(); + s.disconnect(); + work(200); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "--cpu-prof", "--cpu-prof-dir", String(dir), "child.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + const file = readdirSync(String(dir)).find(f => f.endsWith(".cpuprofile")); + expect({ stderr, exitCode, hasFile: !!file }).toEqual({ stderr: "", exitCode: 0, hasFile: true }); + const profile = JSON.parse(readFileSync(`${dir}/${file}`, "utf8")); + // Before the ownership fix the disconnect() drained and cleared the + // shared buffer, so the at-exit writer emitted an empty stub. + expect(profile.samples.length).toBeGreaterThan(0); + }); + + test.concurrent("concurrent Sessions each get a profile for their own window", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import inspector from "node:inspector"; + ${work} + ${postFn} + const A = new inspector.Session(); A.connect(); + const B = new inspector.Session(); B.connect(); + await post(A, "Profiler.enable"); await post(A, "Profiler.start"); + work(400); + await post(B, "Profiler.enable"); await post(B, "Profiler.start"); + work(300); + const b = (await post(B, "Profiler.stop")).profile; + work(150); + const a = (await post(A, "Profiler.stop")).profile; + B.disconnect(); A.disconnect(); + console.log(JSON.stringify({ + aSamples: a.samples.length, + bSamples: b.samples.length, + bDurationMicros: b.endTime - b.startTime, + })); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stderrIfFailed: exitCode === 0 ? "" : stderr, exitCode }).toEqual({ stderrIfFailed: "", exitCode: 0 }); + const { aSamples, bSamples, bDurationMicros } = JSON.parse(stdout); + // Both profiles are independently populated (previously the first stop + // consumed the shared buffer and the second got -32000 "not started"). + expect(aSamples).toBeGreaterThan(0); + expect(bSamples).toBeGreaterThan(0); + // B ran for ~300ms; before the fix it inherited A's startTime and + // reported the whole ~700ms. Allow generous slack for slow CI. + expect(bDurationMicros).toBeLessThan(550_000); + }); + + test.concurrent("Profiler.disable on one Session does not stop another Session's profile", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import inspector from "node:inspector"; + ${work} + ${postFn} + const A = new inspector.Session(); A.connect(); + const B = new inspector.Session(); B.connect(); + await post(A, "Profiler.enable"); await post(A, "Profiler.start"); + await post(B, "Profiler.enable"); + // B never started a profile, but disabling it used to stop A's. + await post(B, "Profiler.disable"); + B.disconnect(); + work(200); + const a = (await post(A, "Profiler.stop")).profile; + A.disconnect(); + console.log(JSON.stringify({ aSamples: a.samples.length })); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stderrIfFailed: exitCode === 0 ? "" : stderr, exitCode }).toEqual({ stderrIfFailed: "", exitCode: 0 }); + expect(JSON.parse(stdout).aSamples).toBeGreaterThan(0); + }); + }); + + describe("post() without a callback", () => { + test("drops command failures instead of throwing", () => { + const session = new inspector.Session(); + session.connect(); + // Method-not-found and command errors reach the callback but are dropped + // when there is none, matching Node's fire-and-forget post(). + expect(session.post("Runtime.notADomain.foo")).toBeUndefined(); + expect(session.post("Profiler.start")).toBeUndefined(); + session.disconnect(); + }); + + test("still throws for argument validation and not-connected", () => { + const session = new inspector.Session(); + expect(() => session.post("Profiler.enable")).toThrow( + expect.objectContaining({ code: "ERR_INSPECTOR_NOT_CONNECTED" }), + ); + session.connect(); + expect(() => session.post(42 as any)).toThrow(expect.objectContaining({ code: "ERR_INVALID_ARG_TYPE" })); + expect(() => session.post("x", 42 as any)).toThrow(expect.objectContaining({ code: "ERR_INVALID_ARG_TYPE" })); + session.disconnect(); + }); + }); + describe("callback API", () => { test("post() with callback receives result", async () => { const session = new inspector.Session(); @@ -419,41 +563,42 @@ describe("node:inspector", () => { }); describe("unsupported methods", () => { - test("unsupported method throws ERR_INSPECTOR_COMMAND", () => { + test("unsupported method reports ERR_INSPECTOR_COMMAND to the callback", async () => { const session = new inspector.Session(); session.connect(); - expect(() => session.post("Runtime.evaluate")).toThrow( - expect.objectContaining({ code: "ERR_INSPECTOR_COMMAND" }), - ); + const err = await postErr(session, "Runtime.evaluate"); + expect(err).toEqual(expect.objectContaining({ code: "ERR_INSPECTOR_COMMAND" })); session.disconnect(); }); }); describe("precise coverage", () => { - test("startPreciseCoverage requires Profiler.enable", () => { + test("startPreciseCoverage requires Profiler.enable", async () => { const session = new inspector.Session(); session.connect(); - expect(() => session.post("Profiler.startPreciseCoverage")).toThrow("Profiler is not enabled"); - expect(() => session.post("Profiler.stopPreciseCoverage")).toThrow("Profiler is not enabled"); + expect((await postErr(session, "Profiler.startPreciseCoverage"))?.message).toContain("Profiler is not enabled"); + expect((await postErr(session, "Profiler.stopPreciseCoverage"))?.message).toContain("Profiler is not enabled"); session.disconnect(); }); - test("takePreciseCoverage before startPreciseCoverage throws", () => { + test("takePreciseCoverage before startPreciseCoverage reports an error", async () => { const session = new inspector.Session(); session.connect(); session.post("Profiler.enable"); - expect(() => session.post("Profiler.takePreciseCoverage")).toThrow("Precise coverage has not been started."); + const err = await postErr(session, "Profiler.takePreciseCoverage"); + expect(err?.message).toContain("Precise coverage has not been started."); session.disconnect(); }); - test("Profiler.disable stops precise coverage, like V8", () => { + test("Profiler.disable stops precise coverage, like V8", async () => { const session = new inspector.Session(); session.connect(); session.post("Profiler.enable"); session.post("Profiler.startPreciseCoverage", { callCount: true, detailed: true }); session.post("Profiler.disable"); session.post("Profiler.enable"); - expect(() => session.post("Profiler.takePreciseCoverage")).toThrow("Precise coverage has not been started."); + const err = await postErr(session, "Profiler.takePreciseCoverage"); + expect(err?.message).toContain("Precise coverage has not been started."); session.disconnect(); }); diff --git a/test/js/node/inspector/inspector.test.ts b/test/js/node/inspector/inspector.test.ts index 3be3c1812761..d36a117332b5 100644 --- a/test/js/node/inspector/inspector.test.ts +++ b/test/js/node/inspector/inspector.test.ts @@ -608,7 +608,7 @@ test("Runtime.consoleAPICalled encodes -0/NaN/Infinity/bigint as unserializableV } }); -test("Session errors carry Node's ERR_INSPECTOR_* codes and post() validates its arguments", () => { +test("Session errors carry Node's ERR_INSPECTOR_* codes and post() validates its arguments", async () => { const session = new inspector.Session(); expect(() => session.post("Runtime.enable")).toThrow( expect.objectContaining({ code: "ERR_INSPECTOR_NOT_CONNECTED", message: "Session is not connected" }), @@ -632,7 +632,12 @@ test("Session errors carry Node's ERR_INSPECTOR_* codes and post() validates its expect(() => session.post("Runtime.enable", (() => {}) as any, () => {})).toThrow( expect.objectContaining({ code: "ERR_INVALID_ARG_TYPE" }), ); - expect(() => session.post("Nonexistent.domain")).toThrow(expect.objectContaining({ code: "ERR_INSPECTOR_COMMAND" })); + // Command failures (method-not-found, protocol errors) are delivered to the + // callback; without one, Node drops them rather than throwing. + const { promise, resolve } = Promise.withResolvers(); + session.post("Nonexistent.domain", err => resolve(err)); + expect(await promise).toEqual(expect.objectContaining({ code: "ERR_INSPECTOR_COMMAND" })); + expect(session.post("Nonexistent.domain")).toBeUndefined(); session.disconnect(); // connectToMainThread() throws ERR_INSPECTOR_NOT_WORKER on the main thread.