Skip to content

bpo-36601: clarify signal handler comment and remove unnecessary pid check. #12784

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.

Sign up for GitHub

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

Merged
miss-islington merged 2 commits into from
May 10, 2019
Merged

bpo-36601: clarify signal handler comment and remove unnecessary pid check. #12784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
607e4ca
bpo-36601: comment that signals can be caught by any thread
jdemeyer Apr 11, 2019
93b3f39
clarify the wording.
gpshead May 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2019-04-13-16-14-16.bpo-36601.mIgS7t.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
A long-since-meaningless check for ``getpid() == main_pid`` was removed
from Python's internal C signal handler.
25 changes: 10 additions & 15 deletions Modules/signalmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,12 @@ class sigset_t_converter(CConverter):
/*
NOTES ON THE INTERACTION BETWEEN SIGNALS AND THREADS

When threads are supported, we want the following semantics:
We want the following semantics:

- only the main thread can set a signal handler
- only the main thread runs the signal handler
- signals can be delivered to any thread
- any thread can get a signal handler
- signals are only delivered to the main thread

I.e. we don't support "synchronous signals" like SIGFPE (catching
this doesn't make much sense in Python anyway) nor do we support
Expand All @@ -88,17 +89,17 @@ class sigset_t_converter(CConverter):
We still have the problem that in some implementations signals
generated by the keyboard (e.g. SIGINT) are delivered to all
threads (e.g. SGI), while in others (e.g. Solaris) such signals are
delivered to one random thread (an intermediate possibility would
be to deliver it to the main thread -- POSIX?). For now, we have
a working implementation that works in all three cases -- the
handler ignores signals if getpid() isn't the same as in the main
thread. XXX This is a hack.
delivered to one random thread. On Linux, signals are delivered to
the main thread (unless the main thread is blocking the signal, for
example because it's already handling the same signal). Since we
allow signals to be delivered to any thread, this works fine. The
only oddity is that the thread executing the Python signal handler
may not be the thread that received the signal.
*/

#include <sys/types.h> /* For pid_t */
#include "pythread.h"
static unsigned long main_thread;
static pid_t main_pid;
static PyInterpreterState *main_interp;

static volatile struct {
Expand Down Expand Up @@ -326,11 +327,7 @@ signal_handler(int sig_num)
{
int save_errno = errno;

/* See NOTES section above */
if (getpid() == main_pid)
{
trip_signal(sig_num);
}
trip_signal(sig_num);

#ifndef HAVE_SIGACTION
#ifdef SIGCHLD
Expand Down Expand Up @@ -1328,7 +1325,6 @@ PyInit__signal(void)
int i;

main_thread = PyThread_get_thread_ident();
main_pid = getpid();
main_interp = _PyInterpreterState_Get();

/* Create the module and add the functions */
Expand Down Expand Up @@ -1741,7 +1737,6 @@ _PySignal_AfterFork(void)
* the interpreter had an opportunity to call the handlers. issue9535. */
_clear_pending_signals();
main_thread = PyThread_get_thread_ident();
main_pid = getpid();
main_interp = _PyInterpreterState_Get();
}

Expand Down