From 8ade433f5164ec60607c293e24237c899ca74ea6 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 1 Feb 2019 06:40:57 +0800 Subject: [PATCH] lib: move signal event handling into bootstrap/node.js 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: https://github.com/nodejs/node/pull/25859 Reviewed-By: Anna Henningsen --- lib/internal/bootstrap/node.js | 20 +++++++-- lib/internal/process/main_thread_only.js | 53 ++++++++++++------------ 2 files changed, 44 insertions(+), 29 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 701037cb942ff8..7a59b4f369bf54 100644 --- 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 index 42579e9da8acd1..290d2f7e9e2eb8 100644 --- 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 };