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

bpo-30808: Use _Py_atomic API for concurrency-sensitive signal state #2417

Merged
merged 6 commits into from
Jul 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use _Py_atomic API for concurrency-sensitive signal state.
38 changes: 20 additions & 18 deletions Modules/signalmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ static pid_t main_pid;
#endif

static volatile struct {
sig_atomic_t tripped;
_Py_atomic_int tripped;
PyObject *func;
} Handlers[NSIG];

Expand All @@ -113,7 +113,7 @@ static volatile sig_atomic_t wakeup_fd = -1;
#endif

/* Speed up sigcheck() when none tripped */
static volatile sig_atomic_t is_tripped = 0;
static _Py_atomic_int is_tripped;

static PyObject *DefaultHandler;
static PyObject *IgnoreHandler;
Expand Down Expand Up @@ -236,11 +236,13 @@ trip_signal(int sig_num)
int fd;
Py_ssize_t rc;

Handlers[sig_num].tripped = 1;
_Py_atomic_store_relaxed(&Handlers[sig_num].tripped, 1);

/* Set is_tripped after setting .tripped, as it gets
cleared in PyErr_CheckSignals() before .tripped. */
is_tripped = 1;
_Py_atomic_store(&is_tripped, 1);

/* Notify ceval.c */
_PyEval_SignalReceived();

/* And then write to the wakeup fd *after* setting all the globals and
Expand Down Expand Up @@ -461,7 +463,7 @@ signal_signal_impl(PyObject *module, int signalnum, PyObject *handler)
return NULL;
}
old_handler = Handlers[signalnum].func;
Handlers[signalnum].tripped = 0;
_Py_atomic_store_relaxed(&Handlers[signalnum].tripped, 0);
Py_INCREF(handler);
Handlers[signalnum].func = handler;
if (old_handler != NULL)
Expand Down Expand Up @@ -1265,11 +1267,11 @@ PyInit__signal(void)
goto finally;
Py_INCREF(IntHandler);

Handlers[0].tripped = 0;
_Py_atomic_store_relaxed(&Handlers[0].tripped, 0);
for (i = 1; i < NSIG; i++) {
void (*t)(int);
t = PyOS_getsig(i);
Handlers[i].tripped = 0;
_Py_atomic_store_relaxed(&Handlers[i].tripped, 0);
if (t == SIG_DFL)
Handlers[i].func = DefaultHandler;
else if (t == SIG_IGN)
Expand Down Expand Up @@ -1493,7 +1495,7 @@ finisignal(void)

for (i = 1; i < NSIG; i++) {
func = Handlers[i].func;
Handlers[i].tripped = 0;
_Py_atomic_store_relaxed(&Handlers[i].tripped, 0);
Handlers[i].func = NULL;
if (i != SIGINT && func != NULL && func != Py_None &&
func != DefaultHandler && func != IgnoreHandler)
Expand All @@ -1514,7 +1516,7 @@ PyErr_CheckSignals(void)
int i;
PyObject *f;

if (!is_tripped)
if (!_Py_atomic_load(&is_tripped))
return 0;

#ifdef WITH_THREAD
Expand All @@ -1536,24 +1538,24 @@ PyErr_CheckSignals(void)
* we receive a signal i after we zero is_tripped and before we
* check Handlers[i].tripped.
*/
is_tripped = 0;
_Py_atomic_store(&is_tripped, 0);

if (!(f = (PyObject *)PyEval_GetFrame()))
f = Py_None;

for (i = 1; i < NSIG; i++) {
if (Handlers[i].tripped) {
if (_Py_atomic_load_relaxed(&Handlers[i].tripped)) {
PyObject *result = NULL;
PyObject *arglist = Py_BuildValue("(iO)", i, f);
Handlers[i].tripped = 0;
_Py_atomic_store_relaxed(&Handlers[i].tripped, 0);

if (arglist) {
result = PyEval_CallObject(Handlers[i].func,
arglist);
Py_DECREF(arglist);
}
if (!result) {
is_tripped = 1;
_Py_atomic_store(&is_tripped, 1);
return -1;
}

Expand Down Expand Up @@ -1592,12 +1594,12 @@ PyOS_FiniInterrupts(void)
int
PyOS_InterruptOccurred(void)
{
if (Handlers[SIGINT].tripped) {
if (_Py_atomic_load_relaxed(&Handlers[SIGINT].tripped)) {
#ifdef WITH_THREAD
if (PyThread_get_thread_ident() != main_thread)
return 0;
#endif
Handlers[SIGINT].tripped = 0;
_Py_atomic_store_relaxed(&Handlers[SIGINT].tripped, 0);
return 1;
}
return 0;
Expand All @@ -1607,11 +1609,11 @@ static void
_clear_pending_signals(void)
{
int i;
if (!is_tripped)
if (!_Py_atomic_load(&is_tripped))
return;
is_tripped = 0;
_Py_atomic_store(&is_tripped, 0);
for (i = 1; i < NSIG; ++i) {
Handlers[i].tripped = 0;
_Py_atomic_store_relaxed(&Handlers[i].tripped, 0);
}
}

Expand Down