diff --git a/lib/internal/main/mksnapshot.js b/lib/internal/main/mksnapshot.js index 0d00acd6a16..63df8c50087 100644 --- a/lib/internal/main/mksnapshot.js +++ b/lib/internal/main/mksnapshot.js @@ -23,10 +23,10 @@ const { emitWarningSync } = require('internal/process/warning'); const { initializeCallbacks, namespace: { - addSerializeCallback, addDeserializeCallback, isBuildingSnapshot, }, + addAfterUserSerializeCallback, } = require('internal/v8/startup_snapshot'); const { @@ -34,6 +34,7 @@ const { } = require('internal/process/pre_execution'); const path = require('path'); +const { getOptionValue } = require('internal/options'); const supportedModules = new SafeSet(new SafeArrayIterator([ // '_http_agent', @@ -140,22 +141,56 @@ function main() { prepareMainThreadExecution(false, false); initializeCallbacks(); - let stackTraceLimitDesc; - addDeserializeCallback(() => { - if (stackTraceLimitDesc !== undefined) { - ObjectDefineProperty(Error, 'stackTraceLimit', stackTraceLimitDesc); - } - }); - addSerializeCallback(() => { - stackTraceLimitDesc = ObjectGetOwnPropertyDescriptor(Error, 'stackTraceLimit'); + // 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 + // undefined. To prevent users from tripping over this, install Error.stackTraceLimit based on + // --stack-trace-limit by ourselves (which defaults to 10). + // See https://chromium-review.googlesource.com/c/v8/v8/+/3319481 + const initialStackTraceLimitDesc = { + value: getOptionValue('--stack-trace-limit'), + configurable: true, + writable: true, + enumerable: true, + __proto__: null, + }; + ObjectDefineProperty(Error, 'stackTraceLimit', initialStackTraceLimitDesc); - if (stackTraceLimitDesc !== undefined) { + let stackTraceLimitDescToRestore; + // Error.stackTraceLimit needs to be removed during serialization, because when V8 deserializes + // the snapshot, it expects Error.stackTraceLimit to be unset so that it can install it as a new property + // using the value of --stack-trace-limit. + addAfterUserSerializeCallback(() => { + const desc = ObjectGetOwnPropertyDescriptor(Error, 'stackTraceLimit'); + + // If it's modified by users, emit a warning. + if (desc && ( + desc.value !== initialStackTraceLimitDesc.value || + desc.configurable !== initialStackTraceLimitDesc.configurable || + desc.writable !== initialStackTraceLimitDesc.writable || + desc.enumerable !== initialStackTraceLimitDesc.enumerable + )) { + process._rawDebug('Error.stackTraceLimit has been modified by the snapshot builder script.'); // We want to use null-prototype objects to not rely on globally mutable // %Object.prototype%. - ObjectSetPrototypeOf(stackTraceLimitDesc, null); - process._rawDebug('Deleting Error.stackTraceLimit from the snapshot. ' + - 'It will be re-installed after deserialization'); - delete Error.stackTraceLimit; + if (desc.configurable) { + stackTraceLimitDescToRestore = desc; + ObjectSetPrototypeOf(stackTraceLimitDescToRestore, null); + process._rawDebug('It will be preserved after snapshot deserialization and override ' + + '--stack-trace-limit passed into the deserialized application.\n' + + 'To allow --stack-trace-limit override in the deserialized application, ' + + 'delete Error.stackTraceLimit.'); + } else { + process._rawDebug('It is not configurable and will crash the application upon deserialization.\n' + + 'To fix the error, make Error.stackTraceLimit configurable.'); + } + } + + delete Error.stackTraceLimit; + }); + + addDeserializeCallback(() => { + if (stackTraceLimitDescToRestore) { + ObjectDefineProperty(Error, 'stackTraceLimit', stackTraceLimitDescToRestore); } }); diff --git a/lib/internal/v8/startup_snapshot.js b/lib/internal/v8/startup_snapshot.js index 7c789577aec..01204b96239 100644 --- a/lib/internal/v8/startup_snapshot.js +++ b/lib/internal/v8/startup_snapshot.js @@ -58,11 +58,16 @@ function addDeserializeCallback(callback, data) { } const serializeCallbacks = []; +const afterUserSerializeCallbacks = []; // Callbacks to run after user callbacks function runSerializeCallbacks() { while (serializeCallbacks.length > 0) { const { 0: callback, 1: data } = serializeCallbacks.shift(); callback(data); } + while (afterUserSerializeCallbacks.length > 0) { + const { 0: callback, 1: data } = afterUserSerializeCallbacks.shift(); + callback(data); + } } function addSerializeCallback(callback, data) { @@ -71,6 +76,10 @@ function addSerializeCallback(callback, data) { serializeCallbacks.push([callback, data]); } +function addAfterUserSerializeCallback(callback, data) { + afterUserSerializeCallbacks.push([callback, data]); +} + function initializeCallbacks() { // Only run the serialize callbacks in snapshot building mode, otherwise // they throw. @@ -117,4 +126,5 @@ module.exports = { setDeserializeMainFunction, isBuildingSnapshot, }, + addAfterUserSerializeCallback, }; diff --git a/test/fixtures/snapshot/error-stack.js b/test/fixtures/snapshot/error-stack.js new file mode 100644 index 00000000000..96afaec2252 --- /dev/null +++ b/test/fixtures/snapshot/error-stack.js @@ -0,0 +1,24 @@ + +const { + setDeserializeMainFunction, +} = require('v8').startupSnapshot; + +console.log(`During snapshot building, Error.stackTraceLimit =`, Error.stackTraceLimit); +console.log(getError('During snapshot building', 30)); + +setDeserializeMainFunction(() => { + console.log(`After snapshot deserialization, Error.stackTraceLimit =`, Error.stackTraceLimit); + console.log(getError('After snapshot deserialization', 30)); +}); + +function getError(message, depth) { + let counter = 1; + function recurse() { + if (counter++ < depth) { + return recurse(); + } + const error = new Error(message); + return error.stack; + } + return recurse(); +} diff --git a/test/fixtures/snapshot/mutate-error-stack-trace-limit.js b/test/fixtures/snapshot/mutate-error-stack-trace-limit.js new file mode 100644 index 00000000000..e9d704ceb32 --- /dev/null +++ b/test/fixtures/snapshot/mutate-error-stack-trace-limit.js @@ -0,0 +1,44 @@ + +const { + addSerializeCallback, + setDeserializeMainFunction, +} = require('v8').startupSnapshot; +const assert = require('assert'); + +if (process.env.TEST_IN_SERIALIZER) { + addSerializeCallback(checkMutate); +} else { + checkMutate(); +} + +function checkMutate() { + // Check that mutation to Error.stackTraceLimit is effective in the snapshot + // builder script. + assert.strictEqual(typeof Error.stackTraceLimit, 'number'); + Error.stackTraceLimit = 0; + assert.strictEqual(getError('', 30), 'Error'); +} + +setDeserializeMainFunction(() => { + // Check that the mutation is preserved in the deserialized main function. + assert.strictEqual(Error.stackTraceLimit, 0); + assert.strictEqual(getError('', 30), 'Error'); + + // Check that it can still be mutated. + Error.stackTraceLimit = 10; + const error = getError('', 30); + const matches = [...error.matchAll(/at recurse/g)]; + assert.strictEqual(matches.length, 10); +}); + +function getError(message, depth) { + let counter = 1; + function recurse() { + if (counter++ < depth) { + return recurse(); + } + const error = new Error(message); + return error.stack; + } + return recurse(); +} diff --git a/test/parallel/test-snapshot-stack-trace-limit-mutation.js b/test/parallel/test-snapshot-stack-trace-limit-mutation.js new file mode 100644 index 00000000000..fe6c9fbd45c --- /dev/null +++ b/test/parallel/test-snapshot-stack-trace-limit-mutation.js @@ -0,0 +1,46 @@ +'use strict'; + +// This tests mutation to Error.stackTraceLimit in both the snapshot builder script +// and the snapshot main script work. + +require('../common'); +const assert = require('assert'); +const tmpdir = require('../common/tmpdir'); +const fixtures = require('../common/fixtures'); +const { spawnSyncAndAssert, spawnSyncAndExitWithoutError } = require('../common/child_process'); + +const blobPath = tmpdir.resolve('snapshot.blob'); + +function test(additionalArguments = [], additionalEnv = {}) { + tmpdir.refresh(); + // Check the mutation works without --stack-trace-limit. + spawnSyncAndAssert(process.execPath, [ + ...additionalArguments, + '--snapshot-blob', + blobPath, + '--build-snapshot', + fixtures.path('snapshot', 'mutate-error-stack-trace-limit.js'), + ], { + cwd: tmpdir.path, + env: { + ...process.env, + ...additionalEnv, + } + }, { + stderr(output) { + assert.match(output, /Error\.stackTraceLimit has been modified by the snapshot builder script/); + assert.match(output, /It will be preserved after snapshot deserialization/); + } + }); + spawnSyncAndExitWithoutError(process.execPath, [ + '--snapshot-blob', + blobPath, + ], { + cwd: tmpdir.path + }); +} + +test(); +test([], { TEST_IN_SERIALIZER: 1 }); +test(['--stack-trace-limit=50']); +test(['--stack-trace-limit=50'], { TEST_IN_SERIALIZER: 1 }); diff --git a/test/parallel/test-snapshot-stack-trace-limit.js b/test/parallel/test-snapshot-stack-trace-limit.js new file mode 100644 index 00000000000..9ba760a9f23 --- /dev/null +++ b/test/parallel/test-snapshot-stack-trace-limit.js @@ -0,0 +1,46 @@ +'use strict'; + +// This tests Error.stackTraceLimit is fixed up for snapshot-building contexts, +// and can be restored after deserialization. + +require('../common'); +const assert = require('assert'); +const tmpdir = require('../common/tmpdir'); +const fixtures = require('../common/fixtures'); +const { spawnSyncAndAssert } = require('../common/child_process'); + +tmpdir.refresh(); +const blobPath = tmpdir.resolve('snapshot.blob'); +{ + spawnSyncAndAssert(process.execPath, [ + '--stack-trace-limit=50', + '--snapshot-blob', + blobPath, + '--build-snapshot', + fixtures.path('snapshot', 'error-stack.js'), + ], { + cwd: tmpdir.path + }, { + stdout(output) { + assert.match(output, /During snapshot building, Error\.stackTraceLimit = 50/); + const matches = [...output.matchAll(/at recurse/g)]; + assert.strictEqual(matches.length, 30); + } + }); +} + +{ + spawnSyncAndAssert(process.execPath, [ + '--stack-trace-limit=20', + '--snapshot-blob', + blobPath, + ], { + cwd: tmpdir.path + }, { + stdout(output) { + assert.match(output, /After snapshot deserialization, Error\.stackTraceLimit = 20/); + const matches = [...output.matchAll(/at recurse/g)]; + assert.strictEqual(matches.length, 20); + } + }); +}