Skip to content

Commit

Permalink
src: add --disable-sigusr1 to prevent signal i/o thread
Browse files Browse the repository at this point in the history
This commit adds a new flag `--disable-sigusr1` to prevent
the SignalIOThread to be up listening the SIGUSR1 events and
then starting the debugging session.

PR-URL: #56441
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
RafaelGSS authored and aduh95 committed Jan 30, 2025
1 parent df78515 commit 48c813f
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 1 deletion.
14 changes: 14 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,17 @@ Disable the `Object.prototype.__proto__` property. If `mode` is `delete`, the
property is removed entirely. If `mode` is `throw`, accesses to the
property throw an exception with the code `ERR_PROTO_ACCESS`.

### `--disable-sigusr1`

<!-- YAML
added: REPLACEME
-->

> Stability: 1.2 - Release candidate
Disable the ability of starting a debugging session by sending a
`SIGUSR1` signal to the process.

### `--disable-warning=code-or-type`

> Stability: 1.1 - Active development
Expand Down Expand Up @@ -1471,6 +1482,7 @@ added: v7.6.0

Set the `host:port` to be used when the inspector is activated.
Useful when activating the inspector by sending the `SIGUSR1` signal.
Except when [`--disable-sigusr1`][] is passed.

Default host is `127.0.0.1`. If port `0` is specified,
a random available port will be used.
Expand Down Expand Up @@ -3082,6 +3094,7 @@ one is included in the list below.
* `--conditions`, `-C`
* `--diagnostic-dir`
* `--disable-proto`
* `--disable-sigusr1`
* `--disable-warning`
* `--disable-wasm-trap-handler`
* `--dns-result-order`
Expand Down Expand Up @@ -3660,6 +3673,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
[`--build-snapshot`]: #--build-snapshot
[`--cpu-prof-dir`]: #--cpu-prof-dir
[`--diagnostic-dir`]: #--diagnostic-dirdirectory
[`--disable-sigusr1`]: #--disable-sigusr1
[`--env-file-if-exists`]: #--env-file-if-existsconfig
[`--env-file`]: #--env-fileconfig
[`--experimental-addon-modules`]: #--experimental-addon-modules
Expand Down
3 changes: 2 additions & 1 deletion src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,8 @@ inline bool Environment::no_global_search_paths() const {
}

inline bool Environment::should_start_debug_signal_handler() const {
return (flags_ & EnvironmentFlags::kNoStartDebugSignalHandler) == 0;
return ((flags_ & EnvironmentFlags::kNoStartDebugSignalHandler) == 0) &&
!options_->disable_sigusr1;
}

inline bool Environment::no_browser_globals() const {
Expand Down
5 changes: 5 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
" (default: current working directory)",
&EnvironmentOptions::diagnostic_dir,
kAllowedInEnvvar);
AddOption("--disable-sigusr1",
"Disable inspector thread to be listening for SIGUSR1 signal",
&EnvironmentOptions::disable_sigusr1,
kAllowedInEnvvar,
false);
AddOption("--dns-result-order",
"set default value of verbatim in dns.lookup. Options are "
"'ipv4first' (IPv4 addresses are placed before IPv6 addresses) "
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ class EnvironmentOptions : public Options {
bool abort_on_uncaught_exception = false;
std::vector<std::string> conditions;
bool detect_module = true;
bool disable_sigusr1 = false;
bool print_required_tla = false;
bool require_module = true;
std::string dns_result_order;
Expand Down
2 changes: 2 additions & 0 deletions test/fixtures/disable-signal/sigusr1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
console.log('pid is', process.pid);
setInterval(() => {}, 1000);
26 changes: 26 additions & 0 deletions test/parallel/test-disable-sigusr1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict';

const common = require('../common');
const fixtures = require('../common/fixtures');
const { it } = require('node:test');
const assert = require('node:assert');
const { NodeInstance } = require('../common/inspector-helper.js');

common.skipIfInspectorDisabled();

it('should not attach a debugger with SIGUSR1', { skip: common.isWindows }, async () => {
const file = fixtures.path('disable-signal/sigusr1.js');
const instance = new NodeInstance(['--disable-sigusr1'], undefined, file);

instance.on('stderr', common.mustNotCall());
const loggedPid = await new Promise((resolve) => {
instance.on('stdout', (data) => {
const matches = data.match(/pid is (\d+)/);
if (matches) resolve(Number(matches[1]));
});
});

assert.ok(process.kill(instance.pid, 'SIGUSR1'));
assert.strictEqual(loggedPid, instance.pid);
assert.ok(await instance.kill());
});

0 comments on commit 48c813f

Please sign in to comment.