Skip to content

Commit

Permalink
bootstrap: clean up warning setup during serialization
Browse files Browse the repository at this point in the history
PR-URL: #38905
Refs: #35711
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
joyeecheung authored and ruyadorno committed Aug 23, 2022
1 parent 3561514 commit 597a517
Show file tree
Hide file tree
Showing 4 changed files with 214 additions and 11 deletions.
30 changes: 27 additions & 3 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ const {
} = require('internal/errors').codes;
const assert = require('internal/assert');

const {
addSerializeCallback,
isBuildingSnapshot,
} = require('v8').startupSnapshot;

function prepareMainThreadExecution(expandArgv1 = false,
initialzeModules = true) {
refreshRuntimeOptions();
Expand Down Expand Up @@ -169,11 +174,21 @@ function addReadOnlyProcessAlias(name, option, enumerable = true) {

function setupWarningHandler() {
const {
onWarning
onWarning,
resetForSerialization
} = require('internal/process/warning');
if (getOptionValue('--warnings') &&
process.env.NODE_NO_WARNINGS !== '1') {
process.on('warning', onWarning);

// The code above would add the listener back during deserialization,
// if applicable.
if (isBuildingSnapshot()) {
addSerializeCallback(() => {
process.removeListener('warning', onWarning);
resetForSerialization();
});
}
}
}

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

process.on(signal, () => {
function doWriteHeapSnapshot() {
writeHeapSnapshot();
});
}
process.on(signal, doWriteHeapSnapshot);

// The code above would add the listener back during deserialization,
// if applicable.
if (isBuildingSnapshot()) {
addSerializeCallback(() => {
process.removeListener(signal, doWriteHeapSnapshot);
});
}
}

function setupTraceCategoryState() {
Expand Down
28 changes: 20 additions & 8 deletions lib/internal/process/warning.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ let fs;
let fd;
let warningFile;
let options;
let traceWarningHelperShown = false;

function resetForSerialization() {
if (fd !== undefined) {
process.removeListener('exit', closeFdOnExit);
}
fd = undefined;
warningFile = undefined;
traceWarningHelperShown = false;
}

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

function closeFdOnExit() {
try {
fs.closeSync(fd);
} catch {
// Continue regardless of error.
}
}

function writeToFile(message) {
if (fd === undefined) {
fs = require('fs');
Expand All @@ -58,13 +76,7 @@ function writeToFile(message) {
} catch {
return writeOut(message);
}
process.on('exit', () => {
try {
fs.closeSync(fd);
} catch {
// Continue regardless of error.
}
});
process.on('exit', closeFdOnExit);
}
fs.appendFile(fd, `${message}\n`, (err) => {
if (err) {
Expand All @@ -77,7 +89,6 @@ function doEmitWarning(warning) {
process.emit('warning', warning);
}

let traceWarningHelperShown = false;
function onWarning(warning) {
if (!(warning instanceof Error)) return;
const isDeprecation = warning.name === 'DeprecationWarning';
Expand Down Expand Up @@ -179,4 +190,5 @@ module.exports = {
emitWarning,
emitWarningSync,
onWarning,
resetForSerialization,
};
1 change: 1 addition & 0 deletions test/fixtures/snapshot/warning.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
process.emitWarning('test warning');
166 changes: 166 additions & 0 deletions test/parallel/test-snapshot-warning.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
'use strict';

// This tests that the warning handler is cleaned up properly
// 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');
}

const assert = require('assert');
const { spawnSync } = require('child_process');
const tmpdir = require('../common/tmpdir');
const fixtures = require('../common/fixtures');
const path = require('path');
const fs = require('fs');

const warningScript = fixtures.path('snapshot', 'warning.js');
const blobPath = path.join(tmpdir.path, 'snapshot.blob');
const empty = fixtures.path('empty.js');

tmpdir.refresh();
{
console.log('\n# Check snapshot scripts that do not emit warnings.');
let child = spawnSync(process.execPath, [
'--snapshot-blob',
blobPath,
'--build-snapshot',
empty,
], {
cwd: tmpdir.path
});
console.log('[stderr]:', child.stderr.toString());
console.log('[stdout]:', child.stdout.toString());
if (child.status !== 0) {
console.log(child.signal);
assert.strictEqual(child.status, 0);
}
const stats = fs.statSync(blobPath);
assert(stats.isFile());

child = spawnSync(process.execPath, [
'--snapshot-blob',
blobPath,
warningScript,
], {
cwd: tmpdir.path
});
console.log('[stderr]:', child.stderr.toString());
console.log('[stdout]:', child.stdout.toString());
if (child.status !== 0) {
console.log(child.signal);
assert.strictEqual(child.status, 0);
}
const match = child.stderr.toString().match(/Warning: test warning/g);
assert.strictEqual(match.length, 1);
}

tmpdir.refresh();
{
console.log('\n# Check snapshot scripts that emit ' +
'warnings and --trace-warnings hint.');
let child = spawnSync(process.execPath, [
'--snapshot-blob',
blobPath,
'--build-snapshot',
warningScript,
], {
cwd: tmpdir.path
});
console.log('[stderr]:', child.stderr.toString());
console.log('[stdout]:', child.stdout.toString());
if (child.status !== 0) {
console.log(child.signal);
assert.strictEqual(child.status, 0);
}
const stats = fs.statSync(blobPath);
assert(stats.isFile());
let match = child.stderr.toString().match(/Warning: test warning/g);
assert.strictEqual(match.length, 1);
match = child.stderr.toString().match(/Use `node --trace-warnings/g);
assert.strictEqual(match.length, 1);

child = spawnSync(process.execPath, [
'--snapshot-blob',
blobPath,
warningScript,
], {
cwd: tmpdir.path
});
console.log('[stderr]:', child.stderr.toString());
console.log('[stdout]:', child.stdout.toString());
if (child.status !== 0) {
console.log(child.signal);
assert.strictEqual(child.status, 0);
}
// Warnings should not be handled more than once.
match = child.stderr.toString().match(/Warning: test warning/g);
assert.strictEqual(match.length, 1);
match = child.stderr.toString().match(/Use `node --trace-warnings/g);
assert.strictEqual(match.length, 1);
}

tmpdir.refresh();
{
console.log('\n# Check --redirect-warnings');
const warningFile1 = path.join(tmpdir.path, 'warnings.txt');
const warningFile2 = path.join(tmpdir.path, 'warnings2.txt');

let child = spawnSync(process.execPath, [
'--snapshot-blob',
blobPath,
'--redirect-warnings',
warningFile1,
'--build-snapshot',
warningScript,
], {
cwd: tmpdir.path
});
console.log('[stderr]:', child.stderr.toString());
console.log('[stdout]:', child.stdout.toString());
if (child.status !== 0) {
console.log(child.signal);
assert.strictEqual(child.status, 0);
}
const stats = fs.statSync(blobPath);
assert(stats.isFile());
const warnings1 = fs.readFileSync(warningFile1, 'utf8');
console.log(warningFile1, ':', warnings1);
let match = warnings1.match(/Warning: test warning/g);
assert.strictEqual(match.length, 1);
match = warnings1.match(/Use `node --trace-warnings/g);
assert.strictEqual(match.length, 1);
assert.doesNotMatch(child.stderr.toString(), /Warning: test warning/);

fs.rmSync(warningFile1, {
maxRetries: 3, recursive: false, force: true
});
child = spawnSync(process.execPath, [
'--snapshot-blob',
blobPath,
'--redirect-warnings',
warningFile2,
warningScript,
], {
cwd: tmpdir.path
});
console.log('[stderr]:', child.stderr.toString());
console.log('[stdout]:', child.stdout.toString());
if (child.status !== 0) {
console.log(child.signal);
assert.strictEqual(child.status, 0);
}
assert(!fs.existsSync(warningFile1));

const warnings2 = fs.readFileSync(warningFile2, 'utf8');
console.log(warningFile2, ':', warnings1);
match = warnings2.match(/Warning: test warning/g);
assert.strictEqual(match.length, 1);
match = warnings2.match(/Use `node --trace-warnings/g);
assert.strictEqual(match.length, 1);
assert.doesNotMatch(child.stderr.toString(), /Warning: test warning/);
}

0 comments on commit 597a517

Please sign in to comment.