Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1af5ad6

Browse filesBrowse files
joyeecheungtargos
joyeecheung
authored and
targos
committed
src: fixup Error.stackTraceLimit during snapshot building
When V8 creates a context for snapshot building, it does not install Error.stackTraceLimit. As a result, error.stack would be undefined in the snapshot builder script unless users explicitly initialize Error.stackTraceLimit, which may be surprising. This patch initializes Error.stackTraceLimit based on the value of --stack-trace-limit to prevent the surprise. If users have modified Error.stackTraceLimit in the builder script, the modified value would be restored during deserialization. Otherwise, the fixed up limit would be deleted since V8 expects to find it unset and re-initialize it during snapshot deserialization. PR-URL: #55121 Fixes: #55100 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent b229083 commit 1af5ad6
Copy full SHA for 1af5ad6

File tree

Expand file treeCollapse file tree

6 files changed

+219
-14
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+219
-14
lines changed
Open diff view settings
Collapse file

‎lib/internal/main/mksnapshot.js‎

Copy file name to clipboardExpand all lines: lib/internal/main/mksnapshot.js
+49-14Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,18 @@ const { emitWarningSync } = require('internal/process/warning');
2323
const {
2424
initializeCallbacks,
2525
namespace: {
26-
addSerializeCallback,
2726
addDeserializeCallback,
2827
isBuildingSnapshot,
2928
},
29+
addAfterUserSerializeCallback,
3030
} = require('internal/v8/startup_snapshot');
3131

3232
const {
3333
prepareMainThreadExecution,
3434
} = require('internal/process/pre_execution');
3535

3636
const path = require('path');
37+
const { getOptionValue } = require('internal/options');
3738

3839
const supportedModules = new SafeSet(new SafeArrayIterator([
3940
// '_http_agent',
@@ -140,22 +141,56 @@ function main() {
140141
prepareMainThreadExecution(false, false);
141142
initializeCallbacks();
142143

143-
let stackTraceLimitDesc;
144-
addDeserializeCallback(() => {
145-
if (stackTraceLimitDesc !== undefined) {
146-
ObjectDefineProperty(Error, 'stackTraceLimit', stackTraceLimitDesc);
144+
// In a context created for building snapshots, V8 does not install Error.stackTraceLimit and as
145+
// a result, if an error is created during the snapshot building process, error.stack would be
146+
// undefined. To prevent users from tripping over this, install Error.stackTraceLimit based on
147+
// --stack-trace-limit by ourselves (which defaults to 10).
148+
// See https://chromium-review.googlesource.com/c/v8/v8/+/3319481
149+
const initialStackTraceLimitDesc = {
150+
value: getOptionValue('--stack-trace-limit'),
151+
configurable: true,
152+
writable: true,
153+
enumerable: true,
154+
__proto__: null,
155+
};
156+
ObjectDefineProperty(Error, 'stackTraceLimit', initialStackTraceLimitDesc);
157+
158+
let stackTraceLimitDescToRestore;
159+
// Error.stackTraceLimit needs to be removed during serialization, because when V8 deserializes
160+
// the snapshot, it expects Error.stackTraceLimit to be unset so that it can install it as a new property
161+
// using the value of --stack-trace-limit.
162+
addAfterUserSerializeCallback(() => {
163+
const desc = ObjectGetOwnPropertyDescriptor(Error, 'stackTraceLimit');
164+
165+
// If it's modified by users, emit a warning.
166+
if (desc && (
167+
desc.value !== initialStackTraceLimitDesc.value ||
168+
desc.configurable !== initialStackTraceLimitDesc.configurable ||
169+
desc.writable !== initialStackTraceLimitDesc.writable ||
170+
desc.enumerable !== initialStackTraceLimitDesc.enumerable
171+
)) {
172+
process._rawDebug('Error.stackTraceLimit has been modified by the snapshot builder script.');
173+
// We want to use null-prototype objects to not rely on globally mutable
174+
// %Object.prototype%.
175+
if (desc.configurable) {
176+
stackTraceLimitDescToRestore = desc;
177+
ObjectSetPrototypeOf(stackTraceLimitDescToRestore, null);
178+
process._rawDebug('It will be preserved after snapshot deserialization and override ' +
179+
'--stack-trace-limit passed into the deserialized application.\n' +
180+
'To allow --stack-trace-limit override in the deserialized application, ' +
181+
'delete Error.stackTraceLimit.');
182+
} else {
183+
process._rawDebug('It is not configurable and will crash the application upon deserialization.\n' +
184+
'To fix the error, make Error.stackTraceLimit configurable.');
185+
}
147186
}
187+
188+
delete Error.stackTraceLimit;
148189
});
149-
addSerializeCallback(() => {
150-
stackTraceLimitDesc = ObjectGetOwnPropertyDescriptor(Error, 'stackTraceLimit');
151190

152-
if (stackTraceLimitDesc !== undefined) {
153-
// We want to use null-prototype objects to not rely on globally mutable
154-
// %Object.prototype%.
155-
ObjectSetPrototypeOf(stackTraceLimitDesc, null);
156-
process._rawDebug('Deleting Error.stackTraceLimit from the snapshot. ' +
157-
'It will be re-installed after deserialization');
158-
delete Error.stackTraceLimit;
191+
addDeserializeCallback(() => {
192+
if (stackTraceLimitDescToRestore) {
193+
ObjectDefineProperty(Error, 'stackTraceLimit', stackTraceLimitDescToRestore);
159194
}
160195
});
161196

Collapse file

‎lib/internal/v8/startup_snapshot.js‎

Copy file name to clipboardExpand all lines: lib/internal/v8/startup_snapshot.js
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,16 @@ function addDeserializeCallback(callback, data) {
5858
}
5959

6060
const serializeCallbacks = [];
61+
const afterUserSerializeCallbacks = []; // Callbacks to run after user callbacks
6162
function runSerializeCallbacks() {
6263
while (serializeCallbacks.length > 0) {
6364
const { 0: callback, 1: data } = serializeCallbacks.shift();
6465
callback(data);
6566
}
67+
while (afterUserSerializeCallbacks.length > 0) {
68+
const { 0: callback, 1: data } = afterUserSerializeCallbacks.shift();
69+
callback(data);
70+
}
6671
}
6772

6873
function addSerializeCallback(callback, data) {
@@ -71,6 +76,10 @@ function addSerializeCallback(callback, data) {
7176
serializeCallbacks.push([callback, data]);
7277
}
7378

79+
function addAfterUserSerializeCallback(callback, data) {
80+
afterUserSerializeCallbacks.push([callback, data]);
81+
}
82+
7483
function initializeCallbacks() {
7584
// Only run the serialize callbacks in snapshot building mode, otherwise
7685
// they throw.
@@ -117,4 +126,5 @@ module.exports = {
117126
setDeserializeMainFunction,
118127
isBuildingSnapshot,
119128
},
129+
addAfterUserSerializeCallback,
120130
};
Collapse file

‎test/fixtures/snapshot/error-stack.js‎

Copy file name to clipboard
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
2+
const {
3+
setDeserializeMainFunction,
4+
} = require('v8').startupSnapshot;
5+
6+
console.log(`During snapshot building, Error.stackTraceLimit =`, Error.stackTraceLimit);
7+
console.log(getError('During snapshot building', 30));
8+
9+
setDeserializeMainFunction(() => {
10+
console.log(`After snapshot deserialization, Error.stackTraceLimit =`, Error.stackTraceLimit);
11+
console.log(getError('After snapshot deserialization', 30));
12+
});
13+
14+
function getError(message, depth) {
15+
let counter = 1;
16+
function recurse() {
17+
if (counter++ < depth) {
18+
return recurse();
19+
}
20+
const error = new Error(message);
21+
return error.stack;
22+
}
23+
return recurse();
24+
}
Collapse file

‎test/fixtures/snapshot/mutate-error-stack-trace-limit.js‎

Copy file name to clipboard
+44Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
2+
const {
3+
addSerializeCallback,
4+
setDeserializeMainFunction,
5+
} = require('v8').startupSnapshot;
6+
const assert = require('assert');
7+
8+
if (process.env.TEST_IN_SERIALIZER) {
9+
addSerializeCallback(checkMutate);
10+
} else {
11+
checkMutate();
12+
}
13+
14+
function checkMutate() {
15+
// Check that mutation to Error.stackTraceLimit is effective in the snapshot
16+
// builder script.
17+
assert.strictEqual(typeof Error.stackTraceLimit, 'number');
18+
Error.stackTraceLimit = 0;
19+
assert.strictEqual(getError('', 30), 'Error');
20+
}
21+
22+
setDeserializeMainFunction(() => {
23+
// Check that the mutation is preserved in the deserialized main function.
24+
assert.strictEqual(Error.stackTraceLimit, 0);
25+
assert.strictEqual(getError('', 30), 'Error');
26+
27+
// Check that it can still be mutated.
28+
Error.stackTraceLimit = 10;
29+
const error = getError('', 30);
30+
const matches = [...error.matchAll(/at recurse/g)];
31+
assert.strictEqual(matches.length, 10);
32+
});
33+
34+
function getError(message, depth) {
35+
let counter = 1;
36+
function recurse() {
37+
if (counter++ < depth) {
38+
return recurse();
39+
}
40+
const error = new Error(message);
41+
return error.stack;
42+
}
43+
return recurse();
44+
}
Collapse file

‎test/parallel/test-snapshot-stack-trace-limit-mutation.js‎

Copy file name to clipboard
+46Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
'use strict';
2+
3+
// This tests mutation to Error.stackTraceLimit in both the snapshot builder script
4+
// and the snapshot main script work.
5+
6+
require('../common');
7+
const assert = require('assert');
8+
const tmpdir = require('../common/tmpdir');
9+
const fixtures = require('../common/fixtures');
10+
const { spawnSyncAndAssert, spawnSyncAndExitWithoutError } = require('../common/child_process');
11+
12+
const blobPath = tmpdir.resolve('snapshot.blob');
13+
14+
function test(additionalArguments = [], additionalEnv = {}) {
15+
tmpdir.refresh();
16+
// Check the mutation works without --stack-trace-limit.
17+
spawnSyncAndAssert(process.execPath, [
18+
...additionalArguments,
19+
'--snapshot-blob',
20+
blobPath,
21+
'--build-snapshot',
22+
fixtures.path('snapshot', 'mutate-error-stack-trace-limit.js'),
23+
], {
24+
cwd: tmpdir.path,
25+
env: {
26+
...process.env,
27+
...additionalEnv,
28+
}
29+
}, {
30+
stderr(output) {
31+
assert.match(output, /Error\.stackTraceLimit has been modified by the snapshot builder script/);
32+
assert.match(output, /It will be preserved after snapshot deserialization/);
33+
}
34+
});
35+
spawnSyncAndExitWithoutError(process.execPath, [
36+
'--snapshot-blob',
37+
blobPath,
38+
], {
39+
cwd: tmpdir.path
40+
});
41+
}
42+
43+
test();
44+
test([], { TEST_IN_SERIALIZER: 1 });
45+
test(['--stack-trace-limit=50']);
46+
test(['--stack-trace-limit=50'], { TEST_IN_SERIALIZER: 1 });
Collapse file

‎test/parallel/test-snapshot-stack-trace-limit.js‎

Copy file name to clipboard
+46Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
'use strict';
2+
3+
// This tests Error.stackTraceLimit is fixed up for snapshot-building contexts,
4+
// and can be restored after deserialization.
5+
6+
require('../common');
7+
const assert = require('assert');
8+
const tmpdir = require('../common/tmpdir');
9+
const fixtures = require('../common/fixtures');
10+
const { spawnSyncAndAssert } = require('../common/child_process');
11+
12+
tmpdir.refresh();
13+
const blobPath = tmpdir.resolve('snapshot.blob');
14+
{
15+
spawnSyncAndAssert(process.execPath, [
16+
'--stack-trace-limit=50',
17+
'--snapshot-blob',
18+
blobPath,
19+
'--build-snapshot',
20+
fixtures.path('snapshot', 'error-stack.js'),
21+
], {
22+
cwd: tmpdir.path
23+
}, {
24+
stdout(output) {
25+
assert.match(output, /During snapshot building, Error\.stackTraceLimit = 50/);
26+
const matches = [...output.matchAll(/at recurse/g)];
27+
assert.strictEqual(matches.length, 30);
28+
}
29+
});
30+
}
31+
32+
{
33+
spawnSyncAndAssert(process.execPath, [
34+
'--stack-trace-limit=20',
35+
'--snapshot-blob',
36+
blobPath,
37+
], {
38+
cwd: tmpdir.path
39+
}, {
40+
stdout(output) {
41+
assert.match(output, /After snapshot deserialization, Error\.stackTraceLimit = 20/);
42+
const matches = [...output.matchAll(/at recurse/g)];
43+
assert.strictEqual(matches.length, 20);
44+
}
45+
});
46+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.