Commit 076115d
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: #60434
Fixes: #60423
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>1 parent b8fd0df commit 076115d
4 files changed
Lines changed: 45 additions & 3 deletions
File tree
- lib/internal
- main
- v8
- test/parallel
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -21,7 +21,6 @@ const { emitExperimentalWarning } = require('internal/util'); | |||
| 21 | 21 | const { emitWarningSync } = require('internal/process/warning'); | |
| 22 | 22 | ||
| 23 | 23 | const { | |
| 24 | - initializeCallbacks, | ||
| 25 | 24 | namespace: { | |
| 26 | 25 | addDeserializeCallback, | |
| 27 | 26 | isBuildingSnapshot, | |
@@ -139,7 +138,6 @@ function requireForUserSnapshot(id) { | |||
| 139 | 138 | ||
| 140 | 139 | function main() { | |
| 141 | 140 | prepareMainThreadExecution(false, false); | |
| 142 | - initializeCallbacks(); | ||
| 143 | 141 | ||
| 144 | 142 | // In a context created for building snapshots, V8 does not install Error.stackTraceLimit and as | |
| 145 | 143 | // a result, if an error is created during the snapshot building process, error.stack would be | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -115,8 +115,8 @@ function setDeserializeMainFunction(callback, data) { | |||
| 115 | 115 | }); | |
| 116 | 116 | } | |
| 117 | 117 | ||
| 118 | + initializeCallbacks(); | ||
| 118 | 119 | module.exports = { | |
| 119 | - initializeCallbacks, | ||
| 120 | 120 | runDeserializeCallbacks, | |
| 121 | 121 | throwIfBuildingSnapshot, | |
| 122 | 122 | // Exposed to require('v8').startupSnapshot | |
Lines changed: 21 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,21 @@ | |||
| 1 | + // Flags: --expose-internals --zero-fill-buffers | ||
| 2 | + // Verifies that Buffer.allocUnsafe() allocates initialized memory under --zero-fill-buffers by | ||
| 3 | + // checking the usage count of the relevant native allocator code path. | ||
| 4 | + 'use strict'; | ||
| 5 | + | ||
| 6 | + const common = require('../common'); | ||
| 7 | + if (!common.isDebug) { | ||
| 8 | + common.skip('Only works in debug mode'); | ||
| 9 | + } | ||
| 10 | + const { internalBinding } = require('internal/test/binding'); | ||
| 11 | + const { getGenericUsageCount } = internalBinding('debug'); | ||
| 12 | + const assert = require('assert'); | ||
| 13 | + | ||
| 14 | + const initialUninitializedCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); | ||
| 15 | + const initialZeroFilledCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); | ||
| 16 | + const buffer = Buffer.allocUnsafe(Buffer.poolSize + 1); | ||
| 17 | + assert(buffer.every((b) => b === 0)); | ||
| 18 | + const newUninitializedCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); | ||
| 19 | + const newZeroFilledCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); | ||
| 20 | + assert.strictEqual(newUninitializedCount, initialUninitializedCount); | ||
| 21 | + assert.notStrictEqual(newZeroFilledCount, initialZeroFilledCount); | ||
Lines changed: 23 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,23 @@ | |||
| 1 | + // Flags: --expose-internals | ||
| 2 | + // Verifies that Buffer.allocUnsafe() indeed allocates uninitialized memory by checking | ||
| 3 | + // the usage count of the relevant native allocator code path. | ||
| 4 | + 'use strict'; | ||
| 5 | + | ||
| 6 | + const common = require('../common'); | ||
| 7 | + if (!common.isDebug) { | ||
| 8 | + common.skip('Only works in debug mode'); | ||
| 9 | + } | ||
| 10 | + const { internalBinding } = require('internal/test/binding'); | ||
| 11 | + const { getGenericUsageCount } = internalBinding('debug'); | ||
| 12 | + const assert = require('assert'); | ||
| 13 | + | ||
| 14 | + const initialUninitializedCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); | ||
| 15 | + const initialZeroFilledCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); | ||
| 16 | + Buffer.allocUnsafe(Buffer.poolSize + 1); | ||
| 17 | + // We can't reliably check the contents of the buffer here because the OS or memory allocator | ||
| 18 | + // used might zero-fill memory for us, or they might happen to return reused memory that was | ||
| 19 | + // previously used by a process that zero-filled it. So instead we just check the usage counts. | ||
| 20 | + const newUninitializedCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.Uninitialized'); | ||
| 21 | + const newZeroFilledCount = getGenericUsageCount('NodeArrayBufferAllocator.Allocate.ZeroFilled'); | ||
| 22 | + assert.notStrictEqual(newUninitializedCount, initialUninitializedCount); | ||
| 23 | + assert.strictEqual(newZeroFilledCount, initialZeroFilledCount); | ||
0 commit comments