Skip to content

Commit

Permalink
process: setup signal handler in prepareMainThreadExecution
Browse files Browse the repository at this point in the history
Because this is only necessary in the main thread.
Also removes the rearming of signal events since no
signal event handlers should be created during the execution
of node.js now that we've made the creation of stdout and stderr
streams lazy - this has been demonstrated in the test coverage.

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>
  • Loading branch information
joyeecheung authored and rvagg committed Feb 28, 2019
1 parent 1333dcc commit 57179a0
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 22 deletions.
21 changes: 0 additions & 21 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,36 +320,15 @@ 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,
handleSignal,
report,
syncConfig
} = NativeModule.require('internal/process/report');
process.report = report;
// Download the CLI / ENV config into JS land.
syncConfig(config, false);
if (config.events.includes('signal')) {
process.on(config.signal, handleSignal);
}
}

function setupProcessObject() {
Expand Down
25 changes: 25 additions & 0 deletions lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ let traceEventsAsyncHook;
function prepareMainThreadExecution() {
setupTraceCategoryState();

// Only main thread receives signals.
setupSignalHandlers();

// 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:
Expand All @@ -28,6 +31,28 @@ function prepareMainThreadExecution() {
loadPreloadModules();
}

function setupSignalHandlers() {
const {
createSignalHandlers
} = require('internal/process/main_thread_only');
const {
startListeningIfSignal,
stopListeningIfSignal
} = 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);
}
}
}

function setupTraceCategoryState() {
const {
asyncHooksEnabledInitial,
Expand Down
1 change: 0 additions & 1 deletion lib/internal/process/main_thread_only.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ function createSignalHandlers() {
}

return {
isSignal,
startListeningIfSignal,
stopListeningIfSignal
};
Expand Down

0 comments on commit 57179a0

Please sign in to comment.