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

src: don't reset embeder signal handlers #47188

Merged
merged 1 commit into from Mar 31, 2023

Conversation

dvyukov
Copy link
Contributor

@dvyukov dvyukov commented Mar 21, 2023

The only bad handler value we can inhert from before exec is SIG_IGN
(any actual function pointer is reset to SIG_DFL during exec).
If that's the case, we want to reset it back to SIG_DFL.
However, it's also possible that an embeder (or an LD_PRELOAD-ed
library) has set up own signal handler for own purposes
(e.g. profiling). If that's the case, keep it intact.

Fix #47013

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Mar 21, 2023
src/node.cc Outdated Show resolved Hide resolved
The only bad handler value we can inhert from before exec is SIG_IGN
(any actual function pointer is reset to SIG_DFL during exec).
If that's the case, we want to reset it back to SIG_DFL.
However, it's also possible that an embeder (or an LD_PRELOAD-ed
library) has set up own signal handler for own purposes
(e.g. profiling). If that's the case, keep it intact.

Fix nodejs#47013
@dvyukov
Copy link
Contributor Author

dvyukov commented Mar 24, 2023

@bnoordhuis please re-approve CI run.

@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 24, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 24, 2023
@bnoordhuis bnoordhuis added request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. labels Mar 31, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 31, 2023
@nodejs-github-bot
Copy link
Collaborator

@bnoordhuis bnoordhuis added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 31, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 31, 2023
@nodejs-github-bot nodejs-github-bot merged commit 37af5f5 into nodejs:main Mar 31, 2023
58 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 37af5f5

RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
The only bad handler value we can inhert from before exec is SIG_IGN
(any actual function pointer is reset to SIG_DFL during exec).
If that's the case, we want to reset it back to SIG_DFL.
However, it's also possible that an embeder (or an LD_PRELOAD-ed
library) has set up own signal handler for own purposes
(e.g. profiling). If that's the case, keep it intact.

Fix #47013

PR-URL: #47188
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Apr 6, 2023
RafaelGSS pushed a commit that referenced this pull request Apr 6, 2023
The only bad handler value we can inhert from before exec is SIG_IGN
(any actual function pointer is reset to SIG_DFL during exec).
If that's the case, we want to reset it back to SIG_DFL.
However, it's also possible that an embeder (or an LD_PRELOAD-ed
library) has set up own signal handler for own purposes
(e.g. profiling). If that's the case, keep it intact.

Fix #47013

PR-URL: #47188
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
The only bad handler value we can inhert from before exec is SIG_IGN
(any actual function pointer is reset to SIG_DFL during exec).
If that's the case, we want to reset it back to SIG_DFL.
However, it's also possible that an embeder (or an LD_PRELOAD-ed
library) has set up own signal handler for own purposes
(e.g. profiling). If that's the case, keep it intact.

Fix #47013

PR-URL: #47188
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Apr 8, 2023
The only bad handler value we can inhert from before exec is SIG_IGN
(any actual function pointer is reset to SIG_DFL during exec).
If that's the case, we want to reset it back to SIG_DFL.
However, it's also possible that an embeder (or an LD_PRELOAD-ed
library) has set up own signal handler for own purposes
(e.g. profiling). If that's the case, keep it intact.

Fix #47013

PR-URL: #47188
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
The only bad handler value we can inhert from before exec is SIG_IGN
(any actual function pointer is reset to SIG_DFL during exec).
If that's the case, we want to reset it back to SIG_DFL.
However, it's also possible that an embeder (or an LD_PRELOAD-ed
library) has set up own signal handler for own purposes
(e.g. profiling). If that's the case, keep it intact.

Fix #47013

PR-URL: #47188
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PlatformInit resets signal handlers to SIG_DFL causing crashes
5 participants