Skip to content

Commit b1e739d

Browse files
joyeecheungrvagg
authored andcommitted
process: move initialization of node-report into pre_execution.js
- 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>
1 parent 57179a0 commit b1e739d

File tree

3 files changed

+35
-21
lines changed

3 files changed

+35
-21
lines changed

lib/internal/bootstrap/node.js

-12
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838

3939
const { internalBinding, NativeModule } = loaderExports;
4040
const { Object, Symbol } = primordials;
41-
const { getOptionValue } = NativeModule.require('internal/options');
4241
const config = internalBinding('config');
4342
const { deprecate } = NativeModule.require('internal/util');
4443

@@ -320,17 +319,6 @@ if (process.env.NODE_V8_COVERAGE) {
320319
};
321320
}
322321

323-
if (getOptionValue('--experimental-report')) {
324-
const {
325-
config,
326-
report,
327-
syncConfig
328-
} = NativeModule.require('internal/process/report');
329-
process.report = report;
330-
// Download the CLI / ENV config into JS land.
331-
syncConfig(config, false);
332-
}
333-
334322
function setupProcessObject() {
335323
const EventEmitter = NativeModule.require('events');
336324
const origProcProto = Object.getPrototypeOf(process);

lib/internal/bootstrap/pre_execution.js

+33-9
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ function prepareMainThreadExecution() {
1111
// Only main thread receives signals.
1212
setupSignalHandlers();
1313

14+
// Process initial configurations of node-report, if any.
15+
initializeReport();
16+
initializeReportSignalHandlers(); // Main-thread-only.
17+
1418
// If the process is spawned with env NODE_CHANNEL_FD, it's probably
1519
// spawned by our child_process module, then initialize IPC.
1620
// This attaches some internal event listeners and creates:
@@ -31,6 +35,20 @@ function prepareMainThreadExecution() {
3135
loadPreloadModules();
3236
}
3337

38+
function initializeReport() {
39+
if (!getOptionValue('--experimental-report')) {
40+
return;
41+
}
42+
const {
43+
config,
44+
report,
45+
syncConfig
46+
} = require('internal/process/report');
47+
process.report = report;
48+
// Download the CLI / ENV config into JS land.
49+
syncConfig(config, false);
50+
}
51+
3452
function setupSignalHandlers() {
3553
const {
3654
createSignalHandlers
@@ -41,15 +59,20 @@ function setupSignalHandlers() {
4159
} = createSignalHandlers();
4260
process.on('newListener', startListeningIfSignal);
4361
process.on('removeListener', stopListeningIfSignal);
62+
}
4463

45-
if (getOptionValue('--experimental-report')) {
46-
const {
47-
config,
48-
handleSignal
49-
} = require('internal/process/report');
50-
if (config.events.includes('signal')) {
51-
process.on(config.signal, handleSignal);
52-
}
64+
// This has to be called after both initializeReport() and
65+
// setupSignalHandlers() are called
66+
function initializeReportSignalHandlers() {
67+
if (!getOptionValue('--experimental-report')) {
68+
return;
69+
}
70+
const {
71+
config,
72+
handleSignal
73+
} = require('internal/process/report');
74+
if (config.events.includes('signal')) {
75+
process.on(config.signal, handleSignal);
5376
}
5477
}
5578

@@ -204,5 +227,6 @@ module.exports = {
204227
initializeDeprecations,
205228
initializeESMLoader,
206229
loadPreloadModules,
207-
setupTraceCategoryState
230+
setupTraceCategoryState,
231+
initializeReport
208232
};

lib/internal/main/worker_thread.js

+2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
const {
77
initializeDeprecations,
88
initializeESMLoader,
9+
initializeReport,
910
loadPreloadModules,
1011
setupTraceCategoryState
1112
} = require('internal/bootstrap/pre_execution');
@@ -75,6 +76,7 @@ port.on('message', (message) => {
7576
} = message;
7677

7778
setupTraceCategoryState();
79+
initializeReport();
7880
if (manifestSrc) {
7981
require('internal/process/policy').setup(manifestSrc, manifestURL);
8082
}

0 commit comments

Comments
 (0)