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

csighandler3: try a fallback interpreter if none in TLS #22530

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

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Aug 22, 2024

If an external library, possibly loaded by an XS module, creates a thread, perl has no control over this thread, nor does it create an interpreter for that thread, so if a signal is delivered to that thread my_perl would be NULL, and we would crash trying to safely deliver the signal.

To avoid that uses the saved main interpreter pointer instead.

Since trying to unsafe deliver the signal to the handler from the wrong thread would result in races on the interpreter's data structures I chose not to direct deliver unsafe signals.

Accessing PL_psig_pend[] from the wrong thread isn't great, but to ensure safe handling of that we'd need lockless atomics, which we don't have access to from C99 (and aren't required in C11).

Fixes #22487

If an external library, possibly loaded by an XS module, creates a
thread, perl has no control over this thread, nor does it create an
interpreter for that thread, so if a signal is delivered to that
thread my_perl would be NULL, and we would crash trying to safely
deliver the signal.

To avoid that uses the saved main interpreter pointer instead.

Since trying to unsafe deliver the signal to the handler from the
wrong thread would result in races on the interpreter's data
structures I chose not to direct deliver unsafe signals.

Accessing PL_psig_pend[] from the wrong thread isn't great, but to
ensure safe handling of that we'd need lockless atomics, which we
don't have access to from C99 (and aren't required in C11).

Fixes Perl#22487
@tonycoz tonycoz requested a review from Leont August 22, 2024 00:42
@Leont
Copy link
Contributor

Leont commented Aug 25, 2024

I'm wondering if a pthread_kill based approach wouldn't be better.

@tonycoz
Copy link
Contributor Author

tonycoz commented Aug 28, 2024

I'm wondering if a pthread_kill based approach wouldn't be better.

Maybe, but Win32 already does something like my patch, Ctrl-C events are delivered in a separate thread, and PERL_GET_SIG_CONTEXT aka win32_signal_context() sets the the context for the current thread from PL_curinterp and expects Perl_csighandler3 to deal with it.

Right now I don't see a direct way for us to get the main thread ID/handle. That's something I'd need to add to intrpvar.h to use pthread_kill().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Segmentation fault in Perl_csighandler3 when used XS with threads.
2 participants