lib: move signal event handling into bootstrap/node.js

joyeecheung · targos · commit 8ade433f5164 · 2019-02-10T19:57:05.000+01:00
Moves the `process.on()` and `promise.emit()` calls happened during bootstrap for signal events into `bootstrap/node.js` so it's easier to tell the side effects. Drive-by changes: - Moves the signal event re-arming to a point later during the bootstrap - as early as it were it's unlikely that there could be any existing signal events to re-arm for node-report. - Use a Map instead of an Object for signal wraps since it is used as a deletable dictionary anyway. PR-URL: #25859 Reviewed-By: Anna Henningsen <anna@addaleax.net>
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -67,9 +67,6 @@ process.config = JSON.parse(internalBinding('native_module').config);
 const rawMethods = internalBinding('process_methods');
 // Set up methods and events on the process object for the main thread
 if (isMainThread) {
-  // This depends on process being an event emitter
-  mainThreadSetup.setupSignalHandlers(internalBinding);
-
   process.abort = rawMethods.abort;
   const wrapped = mainThreadSetup.wrapProcessMethods(rawMethods);
   process.umask = wrapped.umask;
@@ -358,6 +355,23 @@ if (process.env.NODE_V8_COVERAGE) {
   };
 }
 
+// Worker threads don't receive signals.
+if (isMainThread) {
+  const {
+    isSignal,
+    startListeningIfSignal,
+    stopListeningIfSignal
+  } = mainThreadSetup.createSignalHandlers();
+  process.on('newListener', startListeningIfSignal);
+  process.on('removeListener', stopListeningIfSignal);
+  // re-arm pre-existing signal event registrations
+  // with this signal wrap capabilities.
+  const signalEvents = process.eventNames().filter(isSignal);
+  for (const ev of signalEvents) {
+    process.emit('newListener', ev);
+  }
+}
+
 if (getOptionValue('--experimental-report')) {
   const {
     config,
diff --git a/lib/internal/process/main_thread_only.js b/lib/internal/process/main_thread_only.js
@@ -16,6 +16,8 @@ const {
   validateString
 } = require('internal/validators');
 
+const { signals } = internalBinding('constants').os;
+
 // The execution of this function itself should not cause any side effects.
 function wrapProcessMethods(binding) {
   function chdir(directory) {
@@ -104,19 +106,18 @@ function wrapPosixCredentialSetters(credentials) {
   };
 }
 
-// Worker threads don't receive signals.
-function setupSignalHandlers(internalBinding) {
-  const constants = internalBinding('constants').os.signals;
-  const signalWraps = Object.create(null);
-  let Signal;
+let Signal;
+function isSignal(event) {
+  return typeof event === 'string' && signals[event] !== undefined;
+}
 
-  function isSignal(event) {
-    return typeof event === 'string' && constants[event] !== undefined;
-  }
+// Worker threads don't receive signals.
+function createSignalHandlers() {
+  const signalWraps = new Map();
 
   // Detect presence of a listener for the special signal types
-  process.on('newListener', function(type) {
-    if (isSignal(type) && signalWraps[type] === undefined) {
+  function startListeningIfSignal(type) {
+    if (isSignal(type) && !signalWraps.has(type)) {
       if (Signal === undefined)
         Signal = internalBinding('signal_wrap').Signal;
       const wrap = new Signal();
@@ -125,30 +126,30 @@ function setupSignalHandlers(internalBinding) {
 
       wrap.onsignal = process.emit.bind(process, type, type);
 
-      const signum = constants[type];
+      const signum = signals[type];
       const err = wrap.start(signum);
       if (err) {
         wrap.close();
         throw errnoException(err, 'uv_signal_start');
       }
 
-      signalWraps[type] = wrap;
+      signalWraps.set(type, wrap);
     }
-  });
+  }
 
-  process.on('removeListener', function(type) {
-    if (signalWraps[type] !== undefined && this.listenerCount(type) === 0) {
-      signalWraps[type].close();
-      delete signalWraps[type];
+  function stopListeningIfSignal(type) {
+    const wrap = signalWraps.get(type);
+    if (wrap !== undefined && process.listenerCount(type) === 0) {
+      wrap.close();
+      signalWraps.delete(type);
     }
-  });
-
-  // re-arm pre-existing signal event registrations
-  // with this signal wrap capabilities.
-  process.eventNames().forEach((ev) => {
-    if (isSignal(ev))
-      process.emit('newListener', ev);
-  });
+  }
+
+  return {
+    isSignal,
+    startListeningIfSignal,
+    stopListeningIfSignal
+  };
 }
 
 function setupChildProcessIpcChannel() {
@@ -166,7 +167,7 @@ function setupChildProcessIpcChannel() {
 
 module.exports = {
   wrapProcessMethods,
-  setupSignalHandlers,
+  createSignalHandlers,
   setupChildProcessIpcChannel,
   wrapPosixCredentialSetters
 };
