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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
nodejs · Feb 24, 2019 · b0869c6 · b0869c6
1 parent 34470b0
commit b0869c6
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 21 deletions.
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -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');
 
@@ -282,17 +281,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);

diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
@@ -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:
@@ -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
@@ -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);
   }
 }
 
@@ -204,5 +227,6 @@ module.exports = {
   initializeDeprecations,
   initializeESMLoader,
   loadPreloadModules,
-  setupTraceCategoryState
+  setupTraceCategoryState,
+  initializeReport
 };
diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js
@@ -6,6 +6,7 @@
 const {
   initializeDeprecations,
   initializeESMLoader,
+  initializeReport,
   loadPreloadModules,
   setupTraceCategoryState
 } = require('internal/bootstrap/pre_execution');
@@ -75,6 +76,7 @@ port.on('message', (message) => {
     } = message;
 
     setupTraceCategoryState();
+    initializeReport();
     if (manifestSrc) {
       require('internal/process/policy').setup(manifestSrc, manifestURL);
     }