Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

process: delay signal handler setup to pre-execution and skip them in workers #26227

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 0 additions & 33 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -282,38 +281,6 @@ 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() {
const EventEmitter = NativeModule.require('events');
const origProcProto = Object.getPrototypeOf(process);
Expand Down
51 changes: 50 additions & 1 deletion lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ let traceEventsAsyncHook;
function prepareMainThreadExecution() {
setupTraceCategoryState();

// 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:
Expand All @@ -28,6 +35,47 @@ 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
} = require('internal/process/main_thread_only');
const {
startListeningIfSignal,
stopListeningIfSignal
} = createSignalHandlers();
process.on('newListener', startListeningIfSignal);
process.on('removeListener', stopListeningIfSignal);
}

// 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);
}
}

function setupTraceCategoryState() {
const {
asyncHooksEnabledInitial,
Expand Down Expand Up @@ -179,5 +227,6 @@ module.exports = {
initializeDeprecations,
initializeESMLoader,
loadPreloadModules,
setupTraceCategoryState
setupTraceCategoryState,
initializeReport
};
2 changes: 2 additions & 0 deletions lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
const {
initializeDeprecations,
initializeESMLoader,
initializeReport,
loadPreloadModules,
setupTraceCategoryState
} = require('internal/bootstrap/pre_execution');
Expand Down Expand Up @@ -75,6 +76,7 @@ port.on('message', (message) => {
} = message;

setupTraceCategoryState();
initializeReport();
if (manifestSrc) {
require('internal/process/policy').setup(manifestSrc, manifestURL);
}
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