Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
101 changes: 82 additions & 19 deletions bindings/profilers/heap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,17 @@ struct HeapProfilerState {
explicit HeapProfilerState(v8::Isolate* isolate) : isolate(isolate) {}

~HeapProfilerState() {
// Uninstall first. By the time we run, the shared_ptr in PerIsolateData is
// already empty (that is what destroyed us), so NearHeapLimit would find no
// state to work with; anything below that can trigger a GC must not be able
// to reach it.
UninstallNearHeapLimitCallback();

auto profiler = isolate->GetHeapProfiler();
if (profiler) {
profiler->StopSamplingHeapProfiler();
}

UninstallNearHeapLimitCallback();
if (async) {
// defer deletion of async when uv_close callback is invoked
uv_close(reinterpret_cast<uv_handle_t*>(async), [](uv_handle_t* handle) {
Expand Down Expand Up @@ -365,6 +370,21 @@ size_t NearHeapLimit(void* data,
auto isolate = v8::Isolate::GetCurrent();
auto state = PerIsolateData::For(isolate)->GetHeapProfilerState();

if (!state) {
// StopSamplingHeapProfiler uninstalls us before dropping the state, so
// normally this cannot happen. The gap is the other destruction path: a
// shared_ptr copy taken by an in-flight NearHeapLimit or InterruptCallback
// can outlive the per-isolate slot — the OOM JS callback calling
// process.exit() erases PerIsolateData while InterruptCallback still holds
// a reference, so ~HeapProfilerState never runs to uninstall us. Decline
// and let V8 do its normal OOM handling.
//
// Deliberately no RemoveNearHeapLimitCallback here: the state that tracked
// the installation is already unreachable, so callbackInstalled cannot be
// cleared, and the only way to get here is a process on its way out.
return current_heap_limit;
}

if (state->insideCallback) {
// Reentrant call detected, try to increase heap limit a bit so that
// previous callback can proceed
Expand Down Expand Up @@ -401,26 +421,41 @@ size_t NearHeapLimit(void* data,
stats.object_count());
}
}
// GetAllocationProfile returns null when V8's sampling heap profiler isn't
// running, and that can happen while this callback is still installed:
// HeapProfilerCleanupHook stops V8's sampler without touching our state, so
// between that hook and the isolate actually going away we stay registered
// with nothing to sample. The heap-limit bookkeeping below still has to run,
// so skip only the profile-dependent work.
std::unique_ptr<v8::AllocationProfile> profile{
isolate->GetHeapProfiler()->GetAllocationProfile()};
state->profile = TranslateAllocationProfileToCpp(profile->GetRootNode());
if (state->dumpProfileOnStderr) {
dumpAllocationProfile(stderr, state->profile.get());
}

if (!state->export_command.empty()) {
ExportProfile(*state);
}
if (profile) {
state->profile = TranslateAllocationProfileToCpp(profile->GetRootNode());
if (state->dumpProfileOnStderr) {
dumpAllocationProfile(stderr, state->profile.get());
}

if (!state->callback.IsEmpty()) {
if (state->callbackMode & kInterruptCallback) {
isolate->RequestInterrupt(InterruptCallback, nullptr);
if (!state->export_command.empty()) {
ExportProfile(*state);
}
if (state->callbackMode & kAsyncCallback) {
uv_async_send(state->async);

if (!state->callback.IsEmpty()) {
if (state->callbackMode & kInterruptCallback) {
isolate->RequestInterrupt(InterruptCallback, nullptr);
}
if (state->callbackMode & kAsyncCallback) {
uv_async_send(state->async);
}
} else {
state->profile.reset();
}
} else {
// Drop any profile retained from an earlier invocation: it is stale, and
// nothing below is going to consume or replace it.
state->profile.reset();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: keep state->profile.reset();, it should not hurt

fprintf(stderr,
"NearHeapLimit: heap profiler is not enabled, no allocation "
"profile to report\n");
}

if (!state->isMainThread) {
Expand Down Expand Up @@ -518,7 +553,21 @@ NAN_METHOD(HeapProfiler::StartSamplingHeapProfiler) {
NAN_METHOD(HeapProfiler::StopSamplingHeapProfiler) {
auto isolate = info.GetIsolate();
isolate->GetHeapProfiler()->StopSamplingHeapProfiler();
PerIsolateData::For(isolate)->GetHeapProfilerState().reset();

// Uninstall explicitly rather than leaving it to ~HeapProfilerState. reset()
// only destroys the state if this is the last reference, and it need not be:
// NearHeapLimit and InterruptCallback both take a shared_ptr copy for the
// duration of the call, so a stop() reached from inside one of them (the
// near-heap-limit JS callback calling heapProfiler.stop(), say) would leave
// the state alive, the destructor unrun, and this callback still registered
// with V8 while the per-isolate slot is already empty. The next
// near-heap-limit GC would then enter NearHeapLimit with no state at all.
// Idempotent: it clears callbackInstalled.
auto& state = PerIsolateData::For(isolate)->GetHeapProfilerState();
if (state) {
state->UninstallNearHeapLimitCallback();
}
state.reset();

// Remove cleanup hook since profiler is explicitly stopped
{
Expand All @@ -541,14 +590,22 @@ NAN_METHOD(HeapProfiler::GetAllocationProfile) {
if (!profile) {
return Nan::ThrowError("Heap profiler is not enabled.");
}
const bool allocations = state->allocations;
// A non-null profile only proves V8's sampling heap profiler is running; it
// does not imply we are the one who started it. Anything else in the process
// (the inspector's HeapProfiler.startSampling, another agent) can enable it
// without ever going through StartSamplingHeapProfiler, in which case there
// is no per-isolate state. Serve the profile without allocation stats rather
// than dereferencing an empty shared_ptr.
const bool allocations = state && state->allocations;
v8::AllocationProfile::Node* root = profile->GetRootNode();
AllocationProfileNodeStatsMap allocation_stats;
if (allocations) {
allocation_stats = BuildAllocationStatsByNodeId(profile->GetSamples());
}

state->OnNewProfile();
if (state) {
state->OnNewProfile();
}
info.GetReturnValue().Set(TranslateAllocationProfile(
root, allocations ? &allocation_stats : nullptr));
}
Expand All @@ -573,7 +630,11 @@ NAN_METHOD(HeapProfiler::MapAllocationProfile) {
return Nan::ThrowError("Heap profiler is not enabled.");
}

state->OnNewProfile();
// As in GetAllocationProfile: V8's profiler may be running without us having
// started it, so there may be no per-isolate state to update.
if (state) {
state->OnNewProfile();
}

auto root = AllocationProfileNodeView::New(profile->GetRootNode());
v8::Local<v8::Value> argv[] = {root};
Expand Down Expand Up @@ -668,7 +729,9 @@ NAN_MODULE_INIT(HeapProfiler::Init) {
void InterruptCallback(v8::Isolate* isolate, void* data) {
v8::HandleScope scope(isolate);
auto state = PerIsolateData::For(isolate)->GetHeapProfilerState();
if (!state->profile) {
// The interrupt is requested from NearHeapLimit but runs later, so
// StopSamplingHeapProfiler() may have dropped the state in between.
if (!state || !state->profile) {
return;
}
v8::Local<v8::Value> argv[1] = {
Expand Down
79 changes: 79 additions & 0 deletions ts/test/heap-foreign-sampler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* Copyright 2026 Datadog, Inc
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

// Runs in a forked process because the failure mode under test is a SIGSEGV,
// which would take the whole mocha run down with it.
//
// V8's sampling heap profiler can be enabled by anything in the process - here
// the inspector's HeapProfiler.startSampling, but equally DevTools or another
// agent. That leaves getAllocationProfile()/mapAllocationProfile() with a live
// V8 profile but no per-isolate HeapProfilerState, since only
// startSamplingHeapProfiler() creates one. Both used to dereference that empty
// shared_ptr.

import * as inspector from 'inspector';

import * as v8HeapProfiler from '../src/heap-profiler-bindings';

function post(
session: inspector.Session,
method: string,
params?: object,
): Promise<void> {
return new Promise((resolve, reject) => {
session.post(method, params, err => (err ? reject(err) : resolve()));
});
}

async function main() {
const session = new inspector.Session();
session.connect();

await post(session, 'HeapProfiler.enable');
await post(session, 'HeapProfiler.startSampling', {samplingInterval: 16384});

// Allocate so the sampler actually has samples to report. Kept modest and
// scoped to this function: the profile only needs a non-empty sample set.
const retained: Array<{i: number; s: string}> = [];
for (let i = 0; i < 20000; i++) {
retained.push({i, s: 'x'.repeat(16)});
}

// pprof never started the heap profiler, so there is no state for this
// isolate. Both of these must return rather than crash.
const profile = v8HeapProfiler.getAllocationProfile();
if (!profile || typeof profile.name !== 'string') {
throw new Error('getAllocationProfile returned an unusable profile');
}

const mapped = v8HeapProfiler.mapAllocationProfile(node => node.name);
if (typeof mapped !== 'string') {
throw new Error('mapAllocationProfile did not invoke the callback');
}

await post(session, 'HeapProfiler.stopSampling');
session.disconnect();
}

main().then(
() => process.exit(0),
err => {
console.error(err);
process.exit(1);
},
);
46 changes: 46 additions & 0 deletions ts/test/test-heap-profiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,52 @@ describe('HeapProfiler', () => {
});
});

describe('foreign heap sampler', () => {
// Regression test: V8's sampling heap profiler can be enabled by something
// other than pprof (inspector, DevTools, another agent). getAllocationProfile
// and mapAllocationProfile then see a live V8 profile with no per-isolate
// state, and used to dereference an empty shared_ptr. Forked because the
// failure is a SIGSEGV.
it('should not crash when V8 heap sampling was enabled outside of pprof', async function () {
this.timeout(30000);

const proc = fork(path.join(__dirname, 'heap-foreign-sampler.js'), {
silent: true,
// Under the asan CI job the child inherits LD_PRELOAD=libasan and runs
// LeakSanitizer at exit. The child ends on process.exit(), so V8's heap
// is never torn down and every live object is reported as leaked,
// failing the child for reasons that have nothing to do with what this
// test checks. ASAN itself stays on, so a genuine memory error in the
// code under test is still caught.
env: {...process.env, LSAN_OPTIONS: 'detect_leaks=0'},
});
let output = '';
proc.stdout?.on('data', chunk => {
output += chunk;
});
proc.stderr?.on('data', chunk => {
output += chunk;
});

await new Promise<void>((resolve, reject) => {
proc.on('error', reject);
// 'close' rather than 'exit': it fires once the piped stdio has been
// drained, so `output` is complete when it lands in the failure message.
proc.on('close', (code, signal) => {
if (code === 0) {
resolve();
} else {
reject(
new Error(
`heap-foreign-sampler exited with code=${code} signal=${signal}\n${output}`,
),
);
}
});
});
});
});

describe('OOMMonitoring', () => {
it('should restore heap limit after v8 recovers from OOM', async function () {
this.timeout(30000);
Expand Down
Loading