Skip to content

Commit 8ade433

Browse files
joyeecheungtargos
authored andcommitted
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: #25859 Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 5de1034 commit 8ade433

File tree

2 files changed

+44
-29
lines changed

2 files changed

+44
-29
lines changed

lib/internal/bootstrap/node.js

+17-3
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,6 @@ process.config = JSON.parse(internalBinding('native_module').config);
6767
const rawMethods = internalBinding('process_methods');
6868
// Set up methods and events on the process object for the main thread
6969
if (isMainThread) {
70-
// This depends on process being an event emitter
71-
mainThreadSetup.setupSignalHandlers(internalBinding);
72-
7370
process.abort = rawMethods.abort;
7471
const wrapped = mainThreadSetup.wrapProcessMethods(rawMethods);
7572
process.umask = wrapped.umask;
@@ -358,6 +355,23 @@ if (process.env.NODE_V8_COVERAGE) {
358355
};
359356
}
360357

358+
// Worker threads don't receive signals.
359+
if (isMainThread) {
360+
const {
361+
isSignal,
362+
startListeningIfSignal,
363+
stopListeningIfSignal
364+
} = mainThreadSetup.createSignalHandlers();
365+
process.on('newListener', startListeningIfSignal);
366+
process.on('removeListener', stopListeningIfSignal);
367+
// re-arm pre-existing signal event registrations
368+
// with this signal wrap capabilities.
369+
const signalEvents = process.eventNames().filter(isSignal);
370+
for (const ev of signalEvents) {
371+
process.emit('newListener', ev);
372+
}
373+
}
374+
361375
if (getOptionValue('--experimental-report')) {
362376
const {
363377
config,

lib/internal/process/main_thread_only.js

+27-26
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ const {
1616
validateString
1717
} = require('internal/validators');
1818

19+
const { signals } = internalBinding('constants').os;
20+
1921
// The execution of this function itself should not cause any side effects.
2022
function wrapProcessMethods(binding) {
2123
function chdir(directory) {
@@ -104,19 +106,18 @@ function wrapPosixCredentialSetters(credentials) {
104106
};
105107
}
106108

107-
// Worker threads don't receive signals.
108-
function setupSignalHandlers(internalBinding) {
109-
const constants = internalBinding('constants').os.signals;
110-
const signalWraps = Object.create(null);
111-
let Signal;
109+
let Signal;
110+
function isSignal(event) {
111+
return typeof event === 'string' && signals[event] !== undefined;
112+
}
112113

113-
function isSignal(event) {
114-
return typeof event === 'string' && constants[event] !== undefined;
115-
}
114+
// Worker threads don't receive signals.
115+
function createSignalHandlers() {
116+
const signalWraps = new Map();
116117

117118
// Detect presence of a listener for the special signal types
118-
process.on('newListener', function(type) {
119-
if (isSignal(type) && signalWraps[type] === undefined) {
119+
function startListeningIfSignal(type) {
120+
if (isSignal(type) && !signalWraps.has(type)) {
120121
if (Signal === undefined)
121122
Signal = internalBinding('signal_wrap').Signal;
122123
const wrap = new Signal();
@@ -125,30 +126,30 @@ function setupSignalHandlers(internalBinding) {
125126

126127
wrap.onsignal = process.emit.bind(process, type, type);
127128

128-
const signum = constants[type];
129+
const signum = signals[type];
129130
const err = wrap.start(signum);
130131
if (err) {
131132
wrap.close();
132133
throw errnoException(err, 'uv_signal_start');
133134
}
134135

135-
signalWraps[type] = wrap;
136+
signalWraps.set(type, wrap);
136137
}
137-
});
138+
}
138139

139-
process.on('removeListener', function(type) {
140-
if (signalWraps[type] !== undefined && this.listenerCount(type) === 0) {
141-
signalWraps[type].close();
142-
delete signalWraps[type];
140+
function stopListeningIfSignal(type) {
141+
const wrap = signalWraps.get(type);
142+
if (wrap !== undefined && process.listenerCount(type) === 0) {
143+
wrap.close();
144+
signalWraps.delete(type);
143145
}
144-
});
145-
146-
// re-arm pre-existing signal event registrations
147-
// with this signal wrap capabilities.
148-
process.eventNames().forEach((ev) => {
149-
if (isSignal(ev))
150-
process.emit('newListener', ev);
151-
});
146+
}
147+
148+
return {
149+
isSignal,
150+
startListeningIfSignal,
151+
stopListeningIfSignal
152+
};
152153
}
153154

154155
function setupChildProcessIpcChannel() {
@@ -166,7 +167,7 @@ function setupChildProcessIpcChannel() {
166167

167168
module.exports = {
168169
wrapProcessMethods,
169-
setupSignalHandlers,
170+
createSignalHandlers,
170171
setupChildProcessIpcChannel,
171172
wrapPosixCredentialSetters
172173
};

0 commit comments

Comments
 (0)