Skip to content

Update init.c for error on --handle-signals=no with multiple threads #58464

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

arnavk23
Copy link

@arnavk23 arnavk23 commented May 19, 2025

This patch adds a check during startup to disallow using --handle-signals=no when multiple threads are configured via JULIA_NUM_THREADS > 1. This avoids segmentation faults due to the GC failing to safepoint properly without signal handling, as detailed in #50278.

@arnavk23 arnavk23 changed the title Update init.c Update init.c for Error on --handle-signals=no with multiple threads May 19, 2025
@arnavk23 arnavk23 changed the title Update init.c for Error on --handle-signals=no with multiple threads Update init.c for error on --handle-signals=no with multiple threads May 19, 2025
Copy link
Member

@vchuravy vchuravy left a comment

Choose a reason for hiding this comment

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

Thanks! No need to open a new PR (x-ref #58463), you could have pushed a new version of your changes.

@arnavk23
Copy link
Author

Yeah but I actually made a mistake while pushing and instead pushed a new pull request. I have closed the previous one.

@giordano
Copy link
Contributor

analyzegc is unhappy:

/cache/build/builder-amdci4-7/julialang/julia-master/src/init.c:560:39: error: Implicit Atomic seq_cst synchronization [concurrency-implicit-atomics,-warnings-as-errors]
  560 |     if (!jl_options.handle_signals && jl_n_threads > 1) {
      |                                       ^
make: *** [Makefile:571: clang-tidy-init] Error 1

@arnavk23
Copy link
Author

So what should I do

@gbaraldi
Copy link
Member

look for how it's used elsewhere, you need to use a jl_atomic_load_relaxed there

@giordano giordano requested a review from vchuravy May 21, 2025 07:53
@arnavk23
Copy link
Author

@gbaraldi Corrected the file..

@JeffBezanson JeffBezanson added the multithreading Base.Threads and related functionality label May 21, 2025
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.

5 participants