Skip to content

Commit

Permalink
bootstrap: fixup Error.stackTraceLimit for user-land snapshot
Browse files Browse the repository at this point in the history
It's difficult for V8 to handle Error.stackTraceLimit in the snapshot,
so delete it from the Error constructor if it's present before
snapshot serialization, and re-install it after deserialization.
In addition try not to touch it from our internals during snapshot
building in the first place by updating
isErrorStackTraceLimitWritable().

PR-URL: #44203
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
joyeecheung authored and ruyadorno committed Aug 23, 2022
1 parent 40b817c commit 87d7845
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 16 deletions.
6 changes: 6 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,12 @@ function lazyBuffer() {
}

function isErrorStackTraceLimitWritable() {
// Do no touch Error.stackTraceLimit as V8 would attempt to install
// it again during deserialization.
if (require('v8').startupSnapshot.isBuildingSnapshot()) {
return false;
}

const desc = ObjectGetOwnPropertyDescriptor(Error, 'stackTraceLimit');
if (desc === undefined) {
return ObjectIsExtensible(Error);
Expand Down
34 changes: 32 additions & 2 deletions lib/internal/main/mksnapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@

const {
Error,
ObjectDefineProperty,
ObjectGetOwnPropertyDescriptor,
ObjectSetPrototypeOf,
SafeArrayIterator,
SafeSet,
SafeArrayIterator
} = primordials;

const binding = internalBinding('mksnapshot');
Expand Down Expand Up @@ -125,7 +128,21 @@ function main() {
const source = readFileSync(file, 'utf-8');
const serializeMainFunction = compileSerializeMain(filename, source);

require('internal/v8/startup_snapshot').initializeCallbacks();
const {
initializeCallbacks,
namespace: {
addSerializeCallback,
addDeserializeCallback,
},
} = require('internal/v8/startup_snapshot');
initializeCallbacks();

let stackTraceLimitDesc;
addDeserializeCallback(() => {
if (stackTraceLimitDesc !== undefined) {
ObjectDefineProperty(Error, 'stackTraceLimit', stackTraceLimitDesc);
}
});

if (getOptionValue('--inspect-brk')) {
internalBinding('inspector').callAndPauseOnStart(
Expand All @@ -134,6 +151,19 @@ function main() {
} else {
serializeMainFunction(requireForUserSnapshot, filename, dirname);
}

addSerializeCallback(() => {
stackTraceLimitDesc = ObjectGetOwnPropertyDescriptor(Error, 'stackTraceLimit');

if (stackTraceLimitDesc !== undefined) {
// 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;
}
});
}

main();
2 changes: 0 additions & 2 deletions lib/internal/v8/startup_snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ function runSerializeCallbacks() {
const { 0: callback, 1: data } = serializeCallbacks.shift();
callback(data);
}
// Remove the hooks from the snapshot.
require('v8').startupSnapshot = undefined;
}

function addSerializeCallback(callback, data) {
Expand Down
6 changes: 6 additions & 0 deletions test/parallel/test-snapshot-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ const fixtures = require('../common/fixtures');
const path = require('path');
const fs = require('fs');

const v8 = require('v8');

// By default it should be false. We'll test that it's true in snapshot
// building mode in the fixture.
assert(!v8.startupSnapshot.isBuildingSnapshot());

tmpdir.refresh();
const blobPath = path.join(tmpdir.path, 'snapshot.blob');
const entry = fixtures.path('snapshot', 'v8-startup-snapshot-api.js');
Expand Down
7 changes: 1 addition & 6 deletions test/parallel/test-snapshot-typescript.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,7 @@

// This tests the TypeScript compiler in the snapshot.

const common = require('../common');

if (process.features.debug) {
common.skip('V8 snapshot does not work with mutated globals yet: ' +
'https://bugs.chromium.org/p/v8/issues/detail?id=12772');
}
require('../common');

const assert = require('assert');
const { spawnSync } = require('child_process');
Expand Down
7 changes: 1 addition & 6 deletions test/parallel/test-snapshot-warning.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@
// during snapshot serialization and installed again during
// deserialization.

const common = require('../common');

if (process.features.debug) {
common.skip('V8 snapshot does not work with mutated globals yet: ' +
'https://bugs.chromium.org/p/v8/issues/detail?id=12772');
}
require('../common');

const assert = require('assert');
const { spawnSync } = require('child_process');
Expand Down

0 comments on commit 87d7845

Please sign in to comment.