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

Add option to only install essential signal handlers #49981

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jonathan-conder-sm
Copy link
Contributor

When embedding Julia, it is useful to opt out of some of the signal handlers (especially SIGINT). But opting out of SIGSEGV makes it very difficult to use multiple Julia threads, since the GC safepoint implementation relies on segv_handler().

This patch provides a third option to embedders, which installs the essential signal handlers but skips the optional ones. It might make sense for this to be the default when embedding, but I've kept the other two options for backwards compatibility.

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

This seems sensible to me. Alas because it was overlooked for so long, there now is a conflict that needs to be resolved first :-/

@vchuravy
Copy link
Member

Related #50278

@vtjnash
Copy link
Member

vtjnash commented Feb 11, 2024

I think only SIGSEGV is actually essential. And SIGUSR2 is only needed on linux (other kernels typically support this functionality natively, and do not require that hack)

@vtjnash vtjnash added the multithreading Base.Threads and related functionality label Feb 11, 2024
@jonathan-conder-sm
Copy link
Contributor Author

I'm happy to change this to only keep SIGSEGV. Hopefully will have time to rebase it as well in the next week or so

When embedding Julia, it is useful to opt out of some of the signal
handlers (especially SIGINT). But opting out of SIGSEGV makes it very
difficult to use multiple Julia threads, since the GC safepoint
implementation relies on segv_handler().

This patch provides a third option to embedders, which installs the
essential signal handlers but skips the optional ones. It might make
sense for this to be the default when embedding, but I've kept the other
two options for backwards compatibility.
@jonathan-conder-sm
Copy link
Contributor Author

Updated now. I haven't tested yet or thought as hard about this as the previous iteration, probably won't get a chance to test until next week.

However, it's a lot simpler now: HANDLE_SIGNALS_ESSENTIAL is the same as HANDLE_SIGNALS_OFF except it still creates an alternate signal stack, and still calls allocate_mach_handler and/or allocate_segv_handler. I guess I should probably also add Windows support...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multithreading Base.Threads and related functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants