From 0f2ecc37489128d93b26515b907a06465ab2a084 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 27 Oct 2025 13:42:30 +0100 Subject: [PATCH 1/4] src: add COUNT_GENERIC_USAGE utility for tests --- src/api/environment.cc | 9 +++++++-- src/node_debug.cc | 24 ++++++++++++++++++++++++ src/node_debug.h | 4 ++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index b50325f0923a69..8126749cc2034b 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -6,6 +6,7 @@ #include "node.h" #include "node_builtins.h" #include "node_context_data.h" +#include "node_debug.h" #include "node_errors.h" #include "node_exit_code.h" #include "node_internals.h" @@ -112,10 +113,13 @@ MaybeLocal PrepareStackTraceCallback(Local context, void* NodeArrayBufferAllocator::Allocate(size_t size) { void* ret; - if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers) + if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers) { + COUNT_GENERIC_USAGE("NodeArrayBufferAllocator.Allocate.ZeroFilled"); ret = allocator_->Allocate(size); - else + } else { + COUNT_GENERIC_USAGE("NodeArrayBufferAllocator.Allocate.Uninitialized"); ret = allocator_->AllocateUninitialized(size); + } if (ret != nullptr) [[likely]] { total_mem_usage_.fetch_add(size, std::memory_order_relaxed); } @@ -123,6 +127,7 @@ void* NodeArrayBufferAllocator::Allocate(size_t size) { } void* NodeArrayBufferAllocator::AllocateUninitialized(size_t size) { + COUNT_GENERIC_USAGE("NodeArrayBufferAllocator.Allocate.Uninitialized"); void* ret = allocator_->AllocateUninitialized(size); if (ret != nullptr) [[likely]] { total_mem_usage_.fetch_add(size, std::memory_order_relaxed); diff --git a/src/node_debug.cc b/src/node_debug.cc index bb0e11747e418c..8dbc79004ff90a 100644 --- a/src/node_debug.cc +++ b/src/node_debug.cc @@ -8,6 +8,7 @@ #include "v8-fast-api-calls.h" #include "v8.h" +#include #include #include #endif // DEBUG @@ -23,9 +24,20 @@ using v8::Number; using v8::Object; using v8::Value; +thread_local std::unordered_map generic_usage_counters; thread_local std::unordered_map v8_fast_api_call_counts; +void CountGenericUsage(const char* counter_name) { + if (generic_usage_counters.find(counter_name) == generic_usage_counters.end()) + generic_usage_counters[counter_name] = 0; + generic_usage_counters[counter_name]++; +} + +int GetGenericUsageCount(const char* counter_name) { + return generic_usage_counters[counter_name]; +} + void TrackV8FastApiCall(FastStringKey key) { v8_fast_api_call_counts[key]++; } @@ -34,6 +46,17 @@ int GetV8FastApiCallCount(FastStringKey key) { return v8_fast_api_call_counts[key]; } +void GetGenericUsageCount(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + if (!args[0]->IsString()) { + env->ThrowError("getGenericUsageCount must be called with a string"); + return; + } + Utf8Value utf8_key(env->isolate(), args[0]); + args.GetReturnValue().Set( + GetGenericUsageCount(utf8_key.ToStringView().data())); +} + void GetV8FastApiCallCount(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); if (!args[0]->IsString()) { @@ -89,6 +112,7 @@ void Initialize(Local target, Local context, void* priv) { SetMethod(context, target, "getV8FastApiCallCount", GetV8FastApiCallCount); + SetMethod(context, target, "getGenericUsageCount", GetGenericUsageCount); SetFastMethod(context, target, "isEven", SlowIsEven, &fast_is_even); SetFastMethod(context, target, "isOdd", SlowIsOdd, &fast_is_odd); } diff --git a/src/node_debug.h b/src/node_debug.h index 32fbf21c813366..6d9e03a6723dd0 100644 --- a/src/node_debug.h +++ b/src/node_debug.h @@ -13,10 +13,14 @@ namespace debug { void TrackV8FastApiCall(FastStringKey key); int GetV8FastApiCallCount(FastStringKey key); +void CountGenericUsage(const char* counter_name); +#define COUNT_GENERIC_USAGE(name) node::debug::CountGenericUsage(name) + #define TRACK_V8_FAST_API_CALL(key) \ node::debug::TrackV8FastApiCall(FastStringKey(key)) #else // !DEBUG #define TRACK_V8_FAST_API_CALL(key) +#define COUNT_GENERIC_USAGE(name) #endif // DEBUG } // namespace debug From 454caf17e8a4e63f76d5119a9a222108975f46b9 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 27 Oct 2025 13:43:09 +0100 Subject: [PATCH 2/4] src: fix timing of snapshot serialize callback Previously the addAfterUserSerailizeCallback() wasn't ready to be used for building the built-in snapshot. This patch initializes the callbacks at the time lib/internal/v8/start_snapshot.js is loaded, so that these callbacks get run correctly when building the built-in snapshot. Currently when building the built-in snapshot, addAfterUserSerializeCallback() is only used by createUnsafeBuffer(), other usages can only come from user-land snapshots, which is covered by tests, but what gets run by the built-in snapshot building process is less visible, and the path used by createUnsafeBuffer() isn't reliably visible in user land either. This adds an internal usage counter in debug builds to verify this path when building the built-in snapshot. --- lib/internal/main/mksnapshot.js | 2 -- lib/internal/v8/startup_snapshot.js | 2 +- ...test-buffer-alloc-unsafe-is-uninitialized.js | 17 +++++++++++++++++ 3 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js diff --git a/lib/internal/main/mksnapshot.js b/lib/internal/main/mksnapshot.js index 63df8c50087aad..b06b1a70bf4939 100644 --- a/lib/internal/main/mksnapshot.js +++ b/lib/internal/main/mksnapshot.js @@ -21,7 +21,6 @@ const { emitExperimentalWarning } = require('internal/util'); const { emitWarningSync } = require('internal/process/warning'); const { - initializeCallbacks, namespace: { addDeserializeCallback, isBuildingSnapshot, @@ -139,7 +138,6 @@ function requireForUserSnapshot(id) { function main() { prepareMainThreadExecution(false, false); - initializeCallbacks(); // In a context created for building snapshots, V8 does not install Error.stackTraceLimit and as // a result, if an error is created during the snapshot building process, error.stack would be diff --git a/lib/internal/v8/startup_snapshot.js b/lib/internal/v8/startup_snapshot.js index 01204b96239406..af5fb07925dc15 100644 --- a/lib/internal/v8/startup_snapshot.js +++ b/lib/internal/v8/startup_snapshot.js @@ -115,8 +115,8 @@ function setDeserializeMainFunction(callback, data) { }); } +initializeCallbacks(); module.exports = { - initializeCallbacks, runDeserializeCallbacks, throwIfBuildingSnapshot, // Exposed to require('v8').startupSnapshot diff --git a/test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js b/test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js new file mode 100644 index 00000000000000..998181d47d84b7 --- /dev/null +++ b/test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js @@ -0,0 +1,17 @@ +// Flags: --expose-internals +// Verifies that Buffer.allocUnsafe() indeed allocates uninitialized memory by checking +// the usage count of the relevant native allocator code path. +'use strict'; + +const common = require('../common'); +if (!common.isDebug) { + common.skip('Only works in debug mode'); +} +const { internalBinding } = require('internal/test/binding'); +const { getGenericUsageCount } = internalBinding('debug'); +const assert = require('assert'); + +const initialCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); +Buffer.allocUnsafe(Buffer.poolSize + 1); +const newCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); +assert.notStrictEqual(newCount, initialCount); From 998b033b61350993e3091082119da9594782152e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 27 Oct 2025 17:42:51 +0100 Subject: [PATCH 3/4] fixup! src: fix timing of snapshot serialize callback --- ...nsafe-is-initialized-with-zero-fill-flag.js | 18 ++++++++++++++++++ ...est-buffer-alloc-unsafe-is-uninitialized.js | 3 +++ 2 files changed, 21 insertions(+) create mode 100644 test/parallel/test-buffer-alloc-unsafe-is-initialized-with-zero-fill-flag.js diff --git a/test/parallel/test-buffer-alloc-unsafe-is-initialized-with-zero-fill-flag.js b/test/parallel/test-buffer-alloc-unsafe-is-initialized-with-zero-fill-flag.js new file mode 100644 index 00000000000000..1d800b44d64669 --- /dev/null +++ b/test/parallel/test-buffer-alloc-unsafe-is-initialized-with-zero-fill-flag.js @@ -0,0 +1,18 @@ +// Flags: --expose-internals --zero-fill-buffers +// Verifies that Buffer.allocUnsafe() allocates initialized memory under --zero-fill-buffers by +// checking the usage count of the relevant native allocator code path. +'use strict'; + +const common = require('../common'); +if (!common.isDebug) { + common.skip('Only works in debug mode'); +} +const { internalBinding } = require('internal/test/binding'); +const { getGenericUsageCount } = internalBinding('debug'); +const assert = require('assert'); + +const initialCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); +const buffer = Buffer.allocUnsafe(Buffer.poolSize + 1); +assert.strictEqual(buffer.every((b) => b === 0), true); +const newCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); +assert.notStrictEqual(newCount, initialCount); diff --git a/test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js b/test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js index 998181d47d84b7..cde814387f0e69 100644 --- a/test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js +++ b/test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js @@ -13,5 +13,8 @@ const assert = require('assert'); const initialCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); Buffer.allocUnsafe(Buffer.poolSize + 1); +// We can't reliably check the contents of the buffer here because the OS or memory allocator +// used might zero-fill memory for us, or they might happen to return reused memory that was +// previously used by a process that zero-filled it. const newCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); assert.notStrictEqual(newCount, initialCount); From d6d46594f8e892b85b13d41542e2d5ec0b67938e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 27 Oct 2025 18:11:18 +0100 Subject: [PATCH 4/4] fixup! fixup! src: fix timing of snapshot serialize callback --- ...alloc-unsafe-is-initialized-with-zero-fill-flag.js | 11 +++++++---- .../test-buffer-alloc-unsafe-is-uninitialized.js | 11 +++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/test/parallel/test-buffer-alloc-unsafe-is-initialized-with-zero-fill-flag.js b/test/parallel/test-buffer-alloc-unsafe-is-initialized-with-zero-fill-flag.js index 1d800b44d64669..075fa94e418457 100644 --- a/test/parallel/test-buffer-alloc-unsafe-is-initialized-with-zero-fill-flag.js +++ b/test/parallel/test-buffer-alloc-unsafe-is-initialized-with-zero-fill-flag.js @@ -11,8 +11,11 @@ const { internalBinding } = require('internal/test/binding'); const { getGenericUsageCount } = internalBinding('debug'); const assert = require('assert'); -const initialCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); +const initialUninitializedCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); +const initialZeroFilledCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); const buffer = Buffer.allocUnsafe(Buffer.poolSize + 1); -assert.strictEqual(buffer.every((b) => b === 0), true); -const newCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); -assert.notStrictEqual(newCount, initialCount); +assert(buffer.every((b) => b === 0)); +const newUninitializedCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); +const newZeroFilledCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); +assert.strictEqual(newUninitializedCount, initialUninitializedCount); +assert.notStrictEqual(newZeroFilledCount, initialZeroFilledCount); diff --git a/test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js b/test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js index cde814387f0e69..557851c8a7c231 100644 --- a/test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js +++ b/test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js @@ -11,10 +11,13 @@ const { internalBinding } = require('internal/test/binding'); const { getGenericUsageCount } = internalBinding('debug'); const assert = require('assert'); -const initialCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); +const initialUninitializedCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); +const initialZeroFilledCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); Buffer.allocUnsafe(Buffer.poolSize + 1); // We can't reliably check the contents of the buffer here because the OS or memory allocator // used might zero-fill memory for us, or they might happen to return reused memory that was -// previously used by a process that zero-filled it. -const newCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); -assert.notStrictEqual(newCount, initialCount); +// previously used by a process that zero-filled it. So instead we just check the usage counts. +const newUninitializedCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); +const newZeroFilledCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); +assert.notStrictEqual(newUninitializedCount, initialUninitializedCount); +assert.strictEqual(newZeroFilledCount, initialZeroFilledCount);