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

Receive pid that signal was sent from #4075

Closed
mehulkar opened this issue Jan 17, 2023 · 6 comments
Closed

Receive pid that signal was sent from #4075

mehulkar opened this issue Jan 17, 2023 · 6 comments

Comments

@mehulkar
Copy link

mehulkar commented Jan 17, 2023

Details

In the code:

process.on('SIGUSR1', () => {})

I would like to receive information about where the signal came from. Currently, the callback only receives the string SIGUSR1 and an int 30. How do I do this?

Node.js version

Not applicable.

Example code

No response

Operating system

macOS

Scope

signal handling

Module and version

Not applicable.

@mehulkar
Copy link
Author

It seems this is where the signal handler is setup here: https://github.com/nodejs/node/blob/66b1356ebcf55e477639a52baba3067afd9ea7a0/deps/uv/src/unix/signal.c#L224-L242

and according to the sigaction docs, it needs to set sa_sigaction and a SA_SIGINFO flag.

-  sa.sa_handler = uv__signal_handler;
-  sa.sa_flags = SA_RESTART;
+  sa.sa_sigaction = uv__signal_handler;
+  sa.sa_flags = SA_SIGINFO;

I have no real understanding of how this works, this is all based on a couple hours of research that I wrote down here. I'm looking for some guidance about:

  1. Whether this is possible to do already, and I'm looking in the wrong place
  2. If it did require a patch to "make it work", where else would I also have to patch such that the process.on callback actually gets the information? (I tried building node with the patch above and it didn't Just Work™)
  3. Is it desirable to add this information into signal handling always or with an option? Changing the callback signature is a breaking change, so I imagine it would need an option.
  4. If I'm right about how this works, is there some history here about why it works this way, or is it entirely dependent on libuv (and should I ask someone there?)

Thank you!

@gireeshpunathil
Copy link
Member

thanks @mehulkar - this is very detailed research work indeed! I will attempt to answer some:

    1. no, this is not already possible.
    1. the PR you referred (src: set SA_SIGINFO flag when using sa_sigaction node#34648) is potentially abandoned because of portability issues - Node.js is supported in many platforms, and some platforms implement signal handling minimally
    1. this is definitely a useful info, and desirable to add it always. However, point 2 poses challenge IMO. Please feel free to resurrect 34648 and see if you can progress.
    1. no idea.
  • /cc @richardlau , who may be able to provide additional insight as well as guidance.

Again, this is a great ground work, and wishing you good luck with working in Node.js!

@richardlau
Copy link
Member

I have suspicions that the AIX behaviour is faulty and that there is an AIX bug there but I was unable to find the right person/group within IBM to pursue any further and now I'm outside of IBM. The behaviour on AIX was an outlier -- all other platforms I tested returned the signal information to the handler (see https://github.com/richardlau/signals).

@preveen-stack
Copy link
Contributor

cc @nodejs/platform-macos PTAL

Copy link

github-actions bot commented May 7, 2024

It seems there has been no activity on this issue for a while, and it is being closed in 30 days. If you believe this issue should remain open, please leave a comment.
If you need further assistance or have questions, you can also search for similar issues on Stack Overflow.
Make sure to look at the README file for the most updated links.

@github-actions github-actions bot added the stale label May 7, 2024
Copy link

github-actions bot commented Jun 7, 2024

It seems there has been no activity on this issue for a while, and it is being closed. If you believe this issue should remain open, please leave a comment.
If you need further assistance or have questions, you can also search for similar issues on Stack Overflow.
Make sure to look at the README file for the most updated links.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants