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 597a517

Browse filesBrowse files
joyeecheungruyadorno
joyeecheung
authored and
ruyadorno
committed
bootstrap: clean up warning setup during serialization
PR-URL: #38905 Refs: #35711 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 3561514 commit 597a517
Copy full SHA for 597a517

File tree

Expand file treeCollapse file tree

4 files changed

+214
-11
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+214
-11
lines changed
Open diff view settings
Collapse file

‎lib/internal/process/pre_execution.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/pre_execution.js
+27-3Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ const {
2828
} = require('internal/errors').codes;
2929
const assert = require('internal/assert');
3030

31+
const {
32+
addSerializeCallback,
33+
isBuildingSnapshot,
34+
} = require('v8').startupSnapshot;
35+
3136
function prepareMainThreadExecution(expandArgv1 = false,
3237
initialzeModules = true) {
3338
refreshRuntimeOptions();
@@ -169,11 +174,21 @@ function addReadOnlyProcessAlias(name, option, enumerable = true) {
169174

170175
function setupWarningHandler() {
171176
const {
172-
onWarning
177+
onWarning,
178+
resetForSerialization
173179
} = require('internal/process/warning');
174180
if (getOptionValue('--warnings') &&
175181
process.env.NODE_NO_WARNINGS !== '1') {
176182
process.on('warning', onWarning);
183+
184+
// The code above would add the listener back during deserialization,
185+
// if applicable.
186+
if (isBuildingSnapshot()) {
187+
addSerializeCallback(() => {
188+
process.removeListener('warning', onWarning);
189+
resetForSerialization();
190+
});
191+
}
177192
}
178193
}
179194

@@ -327,9 +342,18 @@ function initializeHeapSnapshotSignalHandlers() {
327342
require('internal/validators').validateSignalName(signal);
328343
const { writeHeapSnapshot } = require('v8');
329344

330-
process.on(signal, () => {
345+
function doWriteHeapSnapshot() {
331346
writeHeapSnapshot();
332-
});
347+
}
348+
process.on(signal, doWriteHeapSnapshot);
349+
350+
// The code above would add the listener back during deserialization,
351+
// if applicable.
352+
if (isBuildingSnapshot()) {
353+
addSerializeCallback(() => {
354+
process.removeListener(signal, doWriteHeapSnapshot);
355+
});
356+
}
333357
}
334358

335359
function setupTraceCategoryState() {
Collapse file

‎lib/internal/process/warning.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/warning.js
+20-8Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,16 @@ let fs;
2222
let fd;
2323
let warningFile;
2424
let options;
25+
let traceWarningHelperShown = false;
26+
27+
function resetForSerialization() {
28+
if (fd !== undefined) {
29+
process.removeListener('exit', closeFdOnExit);
30+
}
31+
fd = undefined;
32+
warningFile = undefined;
33+
traceWarningHelperShown = false;
34+
}
2535

2636
function lazyOption() {
2737
// This will load `warningFile` only once. If the flag is not set,
@@ -50,6 +60,14 @@ function writeOut(message) {
5060
error(message);
5161
}
5262

63+
function closeFdOnExit() {
64+
try {
65+
fs.closeSync(fd);
66+
} catch {
67+
// Continue regardless of error.
68+
}
69+
}
70+
5371
function writeToFile(message) {
5472
if (fd === undefined) {
5573
fs = require('fs');
@@ -58,13 +76,7 @@ function writeToFile(message) {
5876
} catch {
5977
return writeOut(message);
6078
}
61-
process.on('exit', () => {
62-
try {
63-
fs.closeSync(fd);
64-
} catch {
65-
// Continue regardless of error.
66-
}
67-
});
79+
process.on('exit', closeFdOnExit);
6880
}
6981
fs.appendFile(fd, `${message}\n`, (err) => {
7082
if (err) {
@@ -77,7 +89,6 @@ function doEmitWarning(warning) {
7789
process.emit('warning', warning);
7890
}
7991

80-
let traceWarningHelperShown = false;
8192
function onWarning(warning) {
8293
if (!(warning instanceof Error)) return;
8394
const isDeprecation = warning.name === 'DeprecationWarning';
@@ -179,4 +190,5 @@ module.exports = {
179190
emitWarning,
180191
emitWarningSync,
181192
onWarning,
193+
resetForSerialization,
182194
};
Collapse file

‎test/fixtures/snapshot/warning.js‎

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
process.emitWarning('test warning');
Collapse file

‎test/parallel/test-snapshot-warning.js‎

Copy file name to clipboard
+166Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
'use strict';
2+
3+
// This tests that the warning handler is cleaned up properly
4+
// during snapshot serialization and installed again during
5+
// deserialization.
6+
7+
const common = require('../common');
8+
9+
if (process.features.debug) {
10+
common.skip('V8 snapshot does not work with mutated globals yet: ' +
11+
'https://bugs.chromium.org/p/v8/issues/detail?id=12772');
12+
}
13+
14+
const assert = require('assert');
15+
const { spawnSync } = require('child_process');
16+
const tmpdir = require('../common/tmpdir');
17+
const fixtures = require('../common/fixtures');
18+
const path = require('path');
19+
const fs = require('fs');
20+
21+
const warningScript = fixtures.path('snapshot', 'warning.js');
22+
const blobPath = path.join(tmpdir.path, 'snapshot.blob');
23+
const empty = fixtures.path('empty.js');
24+
25+
tmpdir.refresh();
26+
{
27+
console.log('\n# Check snapshot scripts that do not emit warnings.');
28+
let child = spawnSync(process.execPath, [
29+
'--snapshot-blob',
30+
blobPath,
31+
'--build-snapshot',
32+
empty,
33+
], {
34+
cwd: tmpdir.path
35+
});
36+
console.log('[stderr]:', child.stderr.toString());
37+
console.log('[stdout]:', child.stdout.toString());
38+
if (child.status !== 0) {
39+
console.log(child.signal);
40+
assert.strictEqual(child.status, 0);
41+
}
42+
const stats = fs.statSync(blobPath);
43+
assert(stats.isFile());
44+
45+
child = spawnSync(process.execPath, [
46+
'--snapshot-blob',
47+
blobPath,
48+
warningScript,
49+
], {
50+
cwd: tmpdir.path
51+
});
52+
console.log('[stderr]:', child.stderr.toString());
53+
console.log('[stdout]:', child.stdout.toString());
54+
if (child.status !== 0) {
55+
console.log(child.signal);
56+
assert.strictEqual(child.status, 0);
57+
}
58+
const match = child.stderr.toString().match(/Warning: test warning/g);
59+
assert.strictEqual(match.length, 1);
60+
}
61+
62+
tmpdir.refresh();
63+
{
64+
console.log('\n# Check snapshot scripts that emit ' +
65+
'warnings and --trace-warnings hint.');
66+
let child = spawnSync(process.execPath, [
67+
'--snapshot-blob',
68+
blobPath,
69+
'--build-snapshot',
70+
warningScript,
71+
], {
72+
cwd: tmpdir.path
73+
});
74+
console.log('[stderr]:', child.stderr.toString());
75+
console.log('[stdout]:', child.stdout.toString());
76+
if (child.status !== 0) {
77+
console.log(child.signal);
78+
assert.strictEqual(child.status, 0);
79+
}
80+
const stats = fs.statSync(blobPath);
81+
assert(stats.isFile());
82+
let match = child.stderr.toString().match(/Warning: test warning/g);
83+
assert.strictEqual(match.length, 1);
84+
match = child.stderr.toString().match(/Use `node --trace-warnings/g);
85+
assert.strictEqual(match.length, 1);
86+
87+
child = spawnSync(process.execPath, [
88+
'--snapshot-blob',
89+
blobPath,
90+
warningScript,
91+
], {
92+
cwd: tmpdir.path
93+
});
94+
console.log('[stderr]:', child.stderr.toString());
95+
console.log('[stdout]:', child.stdout.toString());
96+
if (child.status !== 0) {
97+
console.log(child.signal);
98+
assert.strictEqual(child.status, 0);
99+
}
100+
// Warnings should not be handled more than once.
101+
match = child.stderr.toString().match(/Warning: test warning/g);
102+
assert.strictEqual(match.length, 1);
103+
match = child.stderr.toString().match(/Use `node --trace-warnings/g);
104+
assert.strictEqual(match.length, 1);
105+
}
106+
107+
tmpdir.refresh();
108+
{
109+
console.log('\n# Check --redirect-warnings');
110+
const warningFile1 = path.join(tmpdir.path, 'warnings.txt');
111+
const warningFile2 = path.join(tmpdir.path, 'warnings2.txt');
112+
113+
let child = spawnSync(process.execPath, [
114+
'--snapshot-blob',
115+
blobPath,
116+
'--redirect-warnings',
117+
warningFile1,
118+
'--build-snapshot',
119+
warningScript,
120+
], {
121+
cwd: tmpdir.path
122+
});
123+
console.log('[stderr]:', child.stderr.toString());
124+
console.log('[stdout]:', child.stdout.toString());
125+
if (child.status !== 0) {
126+
console.log(child.signal);
127+
assert.strictEqual(child.status, 0);
128+
}
129+
const stats = fs.statSync(blobPath);
130+
assert(stats.isFile());
131+
const warnings1 = fs.readFileSync(warningFile1, 'utf8');
132+
console.log(warningFile1, ':', warnings1);
133+
let match = warnings1.match(/Warning: test warning/g);
134+
assert.strictEqual(match.length, 1);
135+
match = warnings1.match(/Use `node --trace-warnings/g);
136+
assert.strictEqual(match.length, 1);
137+
assert.doesNotMatch(child.stderr.toString(), /Warning: test warning/);
138+
139+
fs.rmSync(warningFile1, {
140+
maxRetries: 3, recursive: false, force: true
141+
});
142+
child = spawnSync(process.execPath, [
143+
'--snapshot-blob',
144+
blobPath,
145+
'--redirect-warnings',
146+
warningFile2,
147+
warningScript,
148+
], {
149+
cwd: tmpdir.path
150+
});
151+
console.log('[stderr]:', child.stderr.toString());
152+
console.log('[stdout]:', child.stdout.toString());
153+
if (child.status !== 0) {
154+
console.log(child.signal);
155+
assert.strictEqual(child.status, 0);
156+
}
157+
assert(!fs.existsSync(warningFile1));
158+
159+
const warnings2 = fs.readFileSync(warningFile2, 'utf8');
160+
console.log(warningFile2, ':', warnings1);
161+
match = warnings2.match(/Warning: test warning/g);
162+
assert.strictEqual(match.length, 1);
163+
match = warnings2.match(/Use `node --trace-warnings/g);
164+
assert.strictEqual(match.length, 1);
165+
assert.doesNotMatch(child.stderr.toString(), /Warning: test warning/);
166+
}

0 commit comments

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