Skip to content

node:inspector: owner-refcount the shared CPU profiler; drop post() command errors without a callback #36019

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 1 commit into from
Closed

node:inspector: owner-refcount the shared CPU profiler; drop post() command errors without a callback #36019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 30 additions & 18 deletions src/js/node/inspector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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.
Expand All @@ -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())
Expand Down
212 changes: 150 additions & 62 deletions src/jsc/bindings/BunCPUProfiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,33 @@

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<ProfilerOwner> 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<JSC::SamplingProfiler::StackTrace> s_retainedTraces;

void setSamplingInterval(int intervalMicroseconds)
{
Expand All @@ -42,25 +64,33 @@

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<WTF::WallTime>().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<WTF::WallTime>().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);
}

Check failure on line 93 in src/jsc/bindings/BunCPUProfiler.cpp

View check run for this annotation

Claude / Claude Code Review

Stale isCPUProfilerRunning() guard in JSWorker.cpp defeats implicit-owner acquisition 

The `if (!Bun::isCPUProfilerRunning()) Bun::startCPUProfiler(...)` guard in `JSWorker.cpp` (jsWorkerPrototypeFunction_startCpuProfileInternalBody, line 818) was not updated for the new semantics: `isCPUProfilerRunning()` now returns true when *any* owner (including an explicit `node:inspector` Session owner) exists, so `worker.startCpuProfile()` skips acquiring the implicit owner and `handle.stop()` then hits the new `s_implicitProfilerOwner == 0` early-return, giving the parent `kEmptyCpuProfil
Comment on lines 89 to 93

@claude claude Bot Jul 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The if (!Bun::isCPUProfilerRunning()) Bun::startCPUProfiler(...) guard in JSWorker.cpp (jsWorkerPrototypeFunction_startCpuProfileInternalBody, line 818) was not updated for the new semantics: isCPUProfilerRunning() now returns true when any owner (including an explicit node:inspector Session owner) exists, so worker.startCpuProfile() skips acquiring the implicit owner and handle.stop() then hits the new s_implicitProfilerOwner == 0 early-return, giving the parent kEmptyCpuProfileJSON — a regression from before this PR. Drop the guard (and the mirror at line 843); startCPUProfiler() is now internally idempotent on s_implicitProfilerOwner.

Extended reasoning...

What the bug is

This PR changes the semantics of two helpers in BunCPUProfiler.cpp:

  • startCPUProfiler() used to mean "unconditionally start the VM sampler"; it now means "acquire the implicit owner (s_implicitProfilerOwner) if not already held".
  • isCPUProfilerRunning() used to read a single s_isProfilerRunning bool; it now returns !s_profilerOwners.isEmpty(), i.e. true when any owner exists — implicit or explicit (a node:inspector Session that called Profiler.start).

The header comment at BunCPUProfiler.h:25-27 explicitly names "per-Worker profiling" as an implicit-owner client. But that caller was not updated. src/jsc/bindings/webcore/JSWorker.cpp:818-819 still does:

if (!Bun::isCPUProfilerRunning())
    Bun::startCPUProfiler(workerCtx.vm());

and the stop side at :843-844:

if (Bun::isCPUProfilerRunning())
    Bun::stopCPUProfiler(workerCtx.vm(), &result, nullptr);

Under the old single-bool model this guard just avoided a redundant re-start. Under the new model it prevents the Worker's implicit owner from ever being acquired whenever any other owner exists on that thread.

Step-by-step trigger

All profiler state is thread_local and postTaskToWorkerGlobalScope runs the lambda on the worker's JS thread, so a Session inside the worker and the parent's worker.startCpuProfile() share the same s_profilerOwners / s_implicitProfilerOwner.

  1. Inside a Worker, user code creates a node:inspector Session and calls Profiler.start. This routes to jsFunction_startCPUProfiler → acquireCPUProfilerOwner, which appends an explicit owner to s_profilerOwners. s_implicitProfilerOwner stays 0.
  2. The parent calls worker.startCpuProfile(). The posted task at JSWorker.cpp:818 evaluates isCPUProfilerRunning() → !s_profilerOwners.isEmpty() → true, so it skips startCPUProfiler(). s_implicitProfilerOwner stays 0.
  3. The parent later calls handle.stop(). The posted task at JSWorker.cpp:843 sees isCPUProfilerRunning() == true and calls stopCPUProfiler().
  4. The new stopCPUProfiler() checks s_implicitProfilerOwner == 0, hits the early-return, and writes *outJSON = WTF::String().
  5. Back in JSWorker.cpp:845-846, result.isEmpty() is true, so the parent receives kEmptyCpuProfileJSON — zero samples.

Why existing code doesn't prevent it

The guard at :818 was correct pre-PR because isCPUProfilerRunning() and startCPUProfiler() operated on the same single bool. Post-PR they operate on different state: the guard checks "any owner" but the call it guards only manipulates the implicit owner. Nothing else on this path touches s_implicitProfilerOwner.

Impact

Before this PR the same sequence returned a populated profile (the old stopCPUProfiler unconditionally drained the shared buffer). After this PR it silently returns an empty stub. This is exactly the Session×implicit-owner interaction the PR is fixing for Session×Session and Session×--cpu-prof, but the per-Worker path — which the PR's own header comment names as an implicit-owner client — is left with a stale guard that defeats it. Per REVIEW.md: "Fix the whole class in the same PR… every caller of a changed helper. Prefer moving the guard into the shared helper."

Fix

Drop both guards — startCPUProfiler() is now internally idempotent on s_implicitProfilerOwner, and stopCPUProfiler() already handles the "not held" case:

// JSWorker.cpp:818
Bun::startCPUProfiler(workerCtx.vm());
// JSWorker.cpp:843
Bun::stopCPUProfiler(workerCtx.vm(), &result, nullptr);


struct ProfileNode {
int id;
Expand Down Expand Up @@ -270,13 +300,11 @@
}

// 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<long long>(s_profilingStartTime);
if (startTimeMicros > 0)
timestamp = static_cast<long long>(startTimeMicros);
else
timestamp = static_cast<long long>(WTF::WallTime::now().secondsSinceEpoch().value() * 1000000.0);

Expand All @@ -289,53 +317,34 @@
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<JSC::SamplingProfiler::StackTrace>& 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<size_t> 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
Expand All @@ -357,8 +366,8 @@
WTF::Vector<int> samples;
WTF::Vector<long long> timeDeltas;

double startTime = s_profilingStartTime;
double lastTime = s_profilingStartTime;
double startTime = startTimeMicros;
double lastTime = startTimeMicros;

for (size_t idx : sortedIndices) {
auto& stackTrace = stackTraces[idx];
Expand Down Expand Up @@ -617,14 +626,14 @@

// 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<WTF::String, FunctionStats> functionStatsMap;

long long totalTimeUs = 0;
int totalSamples = static_cast<int>(stackTraces.size());
int totalSamples = static_cast<int>(sortedIndices.size());

for (size_t idx : sortedIndices) {
auto& stackTrace = stackTraces[idx];
Expand Down Expand Up @@ -928,6 +937,85 @@
}
}

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));
}

Check failure on line 982 in src/jsc/bindings/BunCPUProfiler.cpp

View check run for this annotation

Claude / Claude Code Review

s_retainedTraces UAF: bun:jsc profile() clears m_liveCellPointers while retained traces still reference cells 

The GC-safety of `s_retainedTraces` depends on the SamplingProfiler's `m_liveCellPointers` surviving until the last owner's `clearData()`, but `require('bun:jsc').profile()` (`functionRunProfiler` in `src/jsc/modules/BunJSCModule.h:715-716`, and its `reportFailure` twin at :731-732) calls `samplingProfiler.pause()` + `clearData()` directly, outside the owner-refcount system. If ≥2 owners are live, one releases (populating `s_retainedTraces`), then user code calls `bun:jsc` `profile()`, `clearDat
Comment on lines +973 to +982

@claude claude Bot Jul 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The GC-safety of s_retainedTraces depends on the SamplingProfiler's m_liveCellPointers surviving until the last owner's clearData(), but require('bun:jsc').profile() (functionRunProfiler in src/jsc/modules/BunJSCModule.h:715-716, and its reportFailure twin at :731-732) calls samplingProfiler.pause() + clearData() directly, outside the owner-refcount system. If ≥2 owners are live, one releases (populating s_retainedTraces), then user code calls bun:jsc profile(), clearData() wipes m_liveCellPointers, GC frees a sampled executable, and the remaining owner's buildProfileOutput dereferences a freed cell — before this PR that external clearData() was data-loss only, but s_retainedTraces upgrades it to a use-after-free. functionRunProfiler/reportFailure should route through acquireCPUProfilerOwner/releaseCPUProfilerOwner (or s_retainedTraces must not retain raw cell pointers across GC under an assumption an unrelated public API can violate).

Extended reasoning...

What the bug is

This PR introduces s_retainedTraces — a thread-local Vector<SamplingProfiler::StackTrace> that persists drained traces across calls when more than one profiler owner is live. Each StackFrame inside those traces holds raw ExecutableBase* / JSObject* pointers. The PR's own comment at BunCPUProfiler.cpp:52-57 and the drain site here document the GC-safety invariant: those raw pointers stay live because releaseStackTraces() moves m_stackTraces out but leaves m_liveCellPointers intact, and SamplingProfiler::visit() marks that set during GC — until the last owner calls clearData().

But functionRunProfiler in src/jsc/modules/BunJSCModule.h:712-717 — the user-reachable require('bun:jsc').profile(fn) API — calls samplingProfiler.pause() + samplingProfiler.clearData() directly on the same VM-singleton profiler, entirely outside the new owner-refcount system. Its reportFailure lambda at :727-732 does the same. clearData() clears m_liveCellPointers, breaking the invariant s_retainedTraces depends on.

Code path that triggers it

  1. --cpu-prof (or inspector Session A's Profiler.start) → acquireCPUProfilerOwner — one owner.
  2. Inspector Session B: Profiler.start → second owner.
  3. Session B: Profiler.stop → releaseCPUProfilerOwner. Not the last owner, so it drains releaseStackTraces() into s_retainedTraces and does not call clearData(). s_retainedTraces now holds StackFrames whose executable/callee cells are rooted only via the profiler's m_liveCellPointers.
  4. User code calls require('bun:jsc').profile(fn) → the report (or reportFailure) lambda runs samplingProfiler.clearData(), wiping m_liveCellPointers.
  5. GC runs. A short-lived sampled executable (an IIFE, an inline callback that already returned) whose only root was m_liveCellPointers is collected.
  6. The remaining owner releases (--cpu-prof at process exit, or Session A's Profiler.stop) → releaseCPUProfilerOwner calls buildProfileOutput over s_retainedTraces, which invokes frame.displayName(vm), frame.sourceProviderAndID(), and dynamicDowncast<JSC::FunctionExecutable>(frame.executable) on a freed cell → use-after-free.

Why existing code doesn't prevent it

The DeferGC inside releaseCPUProfilerOwner only covers that single call — it does not span step 3 → step 6. Between those steps, arbitrary JS runs and GC is free to collect. The owner-refcount system is closed: it knows about --cpu-prof, per-Worker profiling, and node:inspector Sessions, but functionRunProfiler is a fourth sibling that touches the same VM-singleton SamplingProfiler and was never wired in. Nothing in BunCPUProfiler.cpp guards against an external clearData().

Why this is new in this PR

Before this PR, stopCPUProfiler held traces only on-stack, under a single DeferGC, then immediately called clearData() itself. An external clearData() from bun:jsc profile() racing with that path caused data loss (an empty profile), never memory corruption. This PR introduces the first cross-call, cross-GC retention of raw StackFrame cell pointers (s_retainedTraces in the !lastOwner branch), which upgrades the pre-existing conflict to a memory-safety bug.

Impact

User-reachable UAF. bun:jsc profile() is public API; --cpu-prof + a node:inspector Session is exactly the concurrent-owner scenario this PR is designed to enable. Per REVIEW.md, memory safety ("Never let a pointer or slice outlive the memory it points into") is the most-blocked category, and "Fix the whole class in the same PR" applies — functionRunProfiler is a sibling site touching the shared SamplingProfiler that the owner-refcount design must account for.

Fix

Route functionRunProfiler (and reportFailure) through Bun::acquireCPUProfilerOwner / Bun::releaseCPUProfilerOwner instead of direct ensureSamplingProfiler/start/pause/clearData. Alternatively, if bun:jsc profile() must keep its own trace-consumption path, have releaseCPUProfilerOwner clear s_retainedTraces whenever it observes m_liveCellPointers may have been cleared externally, or don't retain raw cell pointers past DeferGC at all (e.g. eagerly stringify frame metadata at drain time so s_retainedTraces holds no GC pointers).


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<JSC::SamplingProfiler::StackTrace> 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)
Expand Down
Loading
Loading