Skip to content

doc: note that listening on SIGSEGV & co is unsafe #8410

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.

Sign up for GitHub

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

Closed
addaleax wants to merge 3 commits into from
Closed

doc: note that listening on SIGSEGV & co is unsafe #8410

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4ff4187
doc: note that listening on SIGSEGV & co is unsafe
addaleax Sep 5, 2016
a83799e
[squash] reference manpage for kill(2)
addaleax Sep 8, 2016
46dddc1
[squash] describe what might happen when listening on these signals
addaleax Sep 8, 2016
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
Next Next commit
doc: note that listening on SIGSEGV & co is unsafe 
Note that trying to listen for some signals using `process.on()`
is unsafe in the `process` docs.
  • Loading branch information
@addaleax
addaleax committed Sep 5, 2016
commit 4ff4187e30b0d751b9b1b2369afe098809e22b78
3 changes: 3 additions & 0 deletions doc/api/process.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,9 @@ It is important to take note of the following:
* `SIGKILL` cannot have a listener installed, it will unconditionally terminate
Node.js on all platforms.
* `SIGSTOP` cannot have a listener installed.
* `SIGBUS`, `SIGFPE`, `SIGSEGV` and `SIGILL`, when not raised artificially,
Copy link
Member

@jasnell jasnell Sep 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raised artificially is a bit obscure. I'm not sure the average reader will know that that means.

inherently leave the process in a state from which it is not safe to
attempt to call JS listeners.
Copy link
Contributor

@Fishrock123 Fishrock123 Sep 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does core not call listeners then...?

Copy link
Member Author

@addaleax addaleax Sep 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libuv schedules them to run, but the control flow returns from the signal handler before actually calling anything. As far as I have been able to tell, that means that the offending code will just be executed again, triggering the signal again, etc. in an infinite loop.

Copy link
Member Author

@addaleax addaleax Sep 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fishrock123 sorry, occurs to me that I might have misread your question – node does try to call the listeners in the usual way, and that works perfectly when the signal is artificially generated, as in process.kill(process.pid, 'SIGSEGV').

Copy link
Contributor

@Fishrock123 Fishrock123 Sep 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and that works perfectly when the signal is artificially generated,

Does that imply that we should not allow these signals to trigger when they are not artificial?

Copy link
Member Author

@addaleax addaleax Sep 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that imply that we should not allow these signals to trigger when they are not artificial?

We might be able to do that… libuv would need to set SA_SIGINFO, which comes with this warning in the Linux manpage:

Use of these latter values in sa_flags may be less portable in applications intended for older UNIX implementations.

I am not sure what practical use cases for manually triggering SIGSEGV there are, though.

Copy link
Contributor

@Fishrock123 Fishrock123 Sep 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@addaleax I mean, should these listeners even function if it is not safe?

Copy link
Member Author

@addaleax addaleax Sep 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fishrock123 I would say no, but it’s probably not something worth the trouble of removing…


*Note*: Windows does not support sending signals, but Node.js offers some
emulation with [`process.kill()`][], and [`ChildProcess.kill()`][]. Sending
Expand Down