Skip to content

Commit

Permalink
process: move initialization of node-report into pre_execution.js
Browse files Browse the repository at this point in the history
- Splits signal handler setup code into two functions: one sets up
  `process.on('SIGNAL_NAME')`, another takes care of the signal
  triggers of node-report. Both should only happen on the main thread.
  The latter needs to happen after the node-report configurations
  are read into the process.
- Move the initialization of node-report into pre_execution.js
  because it depends on CLI/environment settings.

PR-URL: #26227
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
joyeecheung authored and rvagg committed Feb 28, 2019
1 parent 57179a0 commit b1e739d
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 21 deletions.
12 changes: 0 additions & 12 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@

const { internalBinding, NativeModule } = loaderExports;
const { Object, Symbol } = primordials;
const { getOptionValue } = NativeModule.require('internal/options');
const config = internalBinding('config');
const { deprecate } = NativeModule.require('internal/util');

Expand Down Expand Up @@ -320,17 +319,6 @@ if (process.env.NODE_V8_COVERAGE) {
};
}

if (getOptionValue('--experimental-report')) {
const {
config,
report,
syncConfig
} = NativeModule.require('internal/process/report');
process.report = report;
// Download the CLI / ENV config into JS land.
syncConfig(config, false);
}

function setupProcessObject() {
const EventEmitter = NativeModule.require('events');
const origProcProto = Object.getPrototypeOf(process);
Expand Down
42 changes: 33 additions & 9 deletions lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ function prepareMainThreadExecution() {
// Only main thread receives signals.
setupSignalHandlers();

// Process initial configurations of node-report, if any.
initializeReport();
initializeReportSignalHandlers(); // Main-thread-only.

// If the process is spawned with env NODE_CHANNEL_FD, it's probably
// spawned by our child_process module, then initialize IPC.
// This attaches some internal event listeners and creates:
Expand All @@ -31,6 +35,20 @@ function prepareMainThreadExecution() {
loadPreloadModules();
}

function initializeReport() {
if (!getOptionValue('--experimental-report')) {
return;
}
const {
config,
report,
syncConfig
} = require('internal/process/report');
process.report = report;
// Download the CLI / ENV config into JS land.
syncConfig(config, false);
}

function setupSignalHandlers() {
const {
createSignalHandlers
Expand All @@ -41,15 +59,20 @@ function setupSignalHandlers() {
} = createSignalHandlers();
process.on('newListener', startListeningIfSignal);
process.on('removeListener', stopListeningIfSignal);
}

if (getOptionValue('--experimental-report')) {
const {
config,
handleSignal
} = require('internal/process/report');
if (config.events.includes('signal')) {
process.on(config.signal, handleSignal);
}
// This has to be called after both initializeReport() and
// setupSignalHandlers() are called
function initializeReportSignalHandlers() {
if (!getOptionValue('--experimental-report')) {
return;
}
const {
config,
handleSignal
} = require('internal/process/report');
if (config.events.includes('signal')) {
process.on(config.signal, handleSignal);
}
}

Expand Down Expand Up @@ -204,5 +227,6 @@ module.exports = {
initializeDeprecations,
initializeESMLoader,
loadPreloadModules,
setupTraceCategoryState
setupTraceCategoryState,
initializeReport
};
2 changes: 2 additions & 0 deletions lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
const {
initializeDeprecations,
initializeESMLoader,
initializeReport,
loadPreloadModules,
setupTraceCategoryState
} = require('internal/bootstrap/pre_execution');
Expand Down Expand Up @@ -75,6 +76,7 @@ port.on('message', (message) => {
} = message;

setupTraceCategoryState();
initializeReport();
if (manifestSrc) {
require('internal/process/policy').setup(manifestSrc, manifestURL);
}
Expand Down

0 comments on commit b1e739d

Please sign in to comment.