From 1f6b681bf2a49b8d45453b53e866caa557367a27 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 27 Oct 2025 13:43:09 +0100 Subject: [PATCH] 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. PR-URL: https://github.com/nodejs/node/pull/60434 Fixes: https://github.com/nodejs/node/issues/60423 Reviewed-By: Matteo Collina Reviewed-By: Antoine du Hamel Reviewed-By: Richard Lau --- lib/internal/main/mksnapshot.js | 2 -- lib/internal/v8/startup_snapshot.js | 2 +- ...safe-is-initialized-with-zero-fill-flag.js | 21 +++++++++++++++++ ...st-buffer-alloc-unsafe-is-uninitialized.js | 23 +++++++++++++++++++ 4 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-buffer-alloc-unsafe-is-initialized-with-zero-fill-flag.js 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 63df8c5008..b06b1a70bf 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 01204b9623..af5fb07925 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-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 0000000000..075fa94e41 --- /dev/null +++ b/test/parallel/test-buffer-alloc-unsafe-is-initialized-with-zero-fill-flag.js @@ -0,0 +1,21 @@ +// 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 initialUninitializedCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); +const initialZeroFilledCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); +const buffer = Buffer.allocUnsafe(Buffer.poolSize + 1); +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 new file mode 100644 index 0000000000..557851c8a7 --- /dev/null +++ b/test/parallel/test-buffer-alloc-unsafe-is-uninitialized.js @@ -0,0 +1,23 @@ +// 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 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. 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);