diff --git a/bindings/profilers/heap.cc b/bindings/profilers/heap.cc index 6106625d..01a80c9c 100644 --- a/bindings/profilers/heap.cc +++ b/bindings/profilers/heap.cc @@ -23,8 +23,10 @@ #include "translate-heap-profile.hh" #include +#include #include #include +#include #include #include @@ -132,6 +134,7 @@ struct HeapProfilerState { v8::Isolate* isolate = nullptr; uint32_t heap_extension_size = 0; + std::optional automatic_heap_extension_size; uint32_t max_heap_extension_count = 0; uint32_t current_heap_extension_count = 0; uv_async_t* async = nullptr; @@ -370,29 +373,36 @@ 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; + constexpr size_t kFallbackExtension = 10 * 1024 * 1024; + size_t extension = state->heap_extension_size; + if (extension == 0) { + if (!state->automatic_heap_extension_size.has_value()) { + // GetAllocationProfile() can allocate and re-enter this callback. Give + // the capture enough room for at most one young generation, as Node.js + // does for its near-OOM heap snapshot callback. In current V8, + // heap_size_limit() is the current old-generation limit plus the maximum + // young-generation size. Cache the delta so nested callbacks do not + // repeatedly collect heap statistics. + v8::HeapStatistics heap_statistics; + isolate->GetHeapStatistics(&heap_statistics); + const size_t total_heap_limit = heap_statistics.heap_size_limit(); + state->automatic_heap_extension_size = + total_heap_limit > current_heap_limit + ? total_heap_limit - current_heap_limit + : kFallbackExtension; + } + extension = *state->automatic_heap_extension_size; } + // V8 requires a value greater than current_heap_limit to continue. Saturate + // instead of overflowing in the defensive extreme case. + const size_t new_heap_limit = + extension > std::numeric_limits::max() - current_heap_limit + ? std::numeric_limits::max() + : current_heap_limit + extension; + if (state->insideCallback) { - // Reentrant call detected, try to increase heap limit a bit so that - // previous callback can proceed - const uint32_t default_heap_extension_size = 10 * 1024 * 1024; - auto extension_size = state->heap_extension_size - ? state->heap_extension_size - : default_heap_extension_size; - return current_heap_limit + extension_size; + return new_heap_limit; } state->insideCallback = true; defer { @@ -472,18 +482,15 @@ size_t NearHeapLimit(void* data, return current_heap_limit + kExtraHeapAllowance + 1; } - size_t new_heap_limit = - current_heap_limit + - ((state->current_heap_extension_count <= state->max_heap_extension_count) - ? state->heap_extension_size - : 0); if (state->current_heap_extension_count >= state->max_heap_extension_count) { // On Node 14, NearLimitCallback is sometimes called many times, without the // process aborting, even when returned limit is not increased. Disable // callback until next call to GetAllocationProfile() state->UninstallNearHeapLimitCallback(); } - return new_heap_limit; + return state->current_heap_extension_count <= state->max_heap_extension_count + ? new_heap_limit + : current_heap_limit; } NAN_METHOD(HeapProfiler::StartSamplingHeapProfiler) { diff --git a/ts/src/heap-profiler.ts b/ts/src/heap-profiler.ts index 1d00d422..8329b33a 100644 --- a/ts/src/heap-profiler.ts +++ b/ts/src/heap-profiler.ts @@ -245,24 +245,26 @@ export const CallbackMode = { /** * Add monitoring for v8 heap, heap profiler must already be started. * When an out of heap memory event occurs: - * - an extension of heap memory of |heapLimitExtensionSize| bytes is - * requested to v8. This extension can occur |maxHeapLimitExtensionCount| - * number of times. If the extension amount is not enough to satisfy - * memory allocation that triggers GC and OOM, process will abort. + * - an extension of |heapLimitExtensionSize| bytes is requested from v8. If + * the size is zero, an automatic extension of one maximum-sized young + * generation is used, with a 10 MiB fallback if it cannot be detected. + * This top-level extension can occur |maxHeapLimitExtensionCount| times. + * Reentrant rescue extensions used to finish an in-progress capture are + * additional and are not included in that count. * - heap profile is dumped as folded stacks on stderr if * |dumpHeapProfileOnSdterr| is true * - heap profile is dumped in temporary file and a new process is spawned * with |exportCommand| arguments and profile path appended at the end. - * - |callback| is called. Callback can be invoked only if - * heapLimitExtensionSize is enough for the process to continue. Invocation - * will be done by a RequestInterrupt if |callbackMode| is Interrupt or Both, - * this might be unsafe since Isolate should not be reentered - * from RequestInterrupt, but this allows to interrupt synchronous code. - * Otherwise the callback is scheduled to be called asynchronously. + * - |callback| is called. Invocation will be done by a RequestInterrupt if + * |callbackMode| is Interrupt or Both, this might be unsafe since Isolate + * should not be reentered from RequestInterrupt, but this allows to + * interrupt synchronous code. Otherwise the callback is scheduled to be + * called asynchronously. * @param heapLimitExtensionSize - amount of bytes heap should be expanded - * with upon OOM + * with upon OOM, or zero to select an automatic young-generation-sized + * extension * @param maxHeapLimitExtensionCount - maximum number of times heap size - * extension can occur + * can be extended after a top-level profile capture * @param dumpHeapProfileOnSdterr - dump heap profile on stderr upon OOM * @param exportCommand - command to execute upon OOM, filepath of a * temporary file containing heap profile will be appended diff --git a/ts/test/oom-restore-heap-limit.ts b/ts/test/oom-restore-heap-limit.ts index c053a03c..6327d773 100644 --- a/ts/test/oom-restore-heap-limit.ts +++ b/ts/test/oom-restore-heap-limit.ts @@ -23,6 +23,7 @@ import {heap} from '../src/index'; const MB = 1024 * 1024; const LIMIT_TOLERANCE = 16 * MB; const CHUNK_SIZE = 4 * MB; +const heapLimitExtensionSize = Number(process.argv[2] || 0); const gc = (global as typeof globalThis & {gc?: () => void}).gc; function heapLimit() { @@ -44,7 +45,7 @@ async function main() { heap.start(MB, 64); try { - heap.monitorOutOfMemory(64 * MB, 1, false); + heap.monitorOutOfMemory(heapLimitExtensionSize, 1, false); const initialLimit = heapLimit(); const retained: number[][] = []; diff --git a/ts/test/oom.ts b/ts/test/oom.ts index d028afa7..9e8f224c 100644 --- a/ts/test/oom.ts +++ b/ts/test/oom.ts @@ -6,7 +6,9 @@ import path from 'path'; const nworkers = Number(process.argv[2] || 0); const workerMaxOldGenerationSizeMb = process.argv[3]; -const maxCount = Number(process.argv[4] || 12); +// Keep the default leak larger than V8's dynamic young-generation allowance so +// the process still reaches its intentional terminal OOM on every Node version. +const maxCount = Number(process.argv[4] || 100); const sleepMs = Number(process.argv[5] || 50); const sizeQuantum = Number(process.argv[6] || 5 * 1024 * 1024); diff --git a/ts/test/test-heap-profiler.ts b/ts/test/test-heap-profiler.ts index 20b62b80..942d65f8 100644 --- a/ts/test/test-heap-profiler.ts +++ b/ts/test/test-heap-profiler.ts @@ -377,13 +377,15 @@ describe('foreign heap sampler', () => { }); describe('OOMMonitoring', () => { - it('should restore heap limit after v8 recovers from OOM', async function () { - this.timeout(30000); - - const proc = fork(path.join(__dirname, 'oom-restore-heap-limit.js'), { - execArgv: ['--expose-gc', '--max-old-space-size=64'], - silent: true, - }); + async function assertHeapLimitIsRestored(heapLimitExtensionSize: number) { + const proc = fork( + path.join(__dirname, 'oom-restore-heap-limit.js'), + [String(heapLimitExtensionSize)], + { + execArgv: ['--expose-gc', '--max-old-space-size=64'], + silent: true, + }, + ); let output = ''; proc.stdout?.on('data', chunk => { @@ -405,6 +407,16 @@ describe('OOMMonitoring', () => { } }); }); + } + + it('should restore an automatic heap limit extension', async function () { + this.timeout(30000); + await assertHeapLimitIsRestored(0); + }); + + it('should restore a configured heap limit extension', async function () { + this.timeout(30000); + await assertHeapLimitIsRestored(64 * 1024 * 1024); }); it('should call external process upon OOM', async function () {