[Python-checkins] [3.6] bpo-30808: Use _Py_atomic API for concurrency-sensitive signal state (GH-2417) (#3007)

[Antoine Pitrou]

https://github.com/python/cpython/commit/57e836cb41656ac23042886db6b5985c036a7ce2
commit: 57e836cb41656ac23042886db6b5985c036a7ce2
branch: 3.6
author: Antoine Pitrou <pitrou at free.fr>
committer: GitHub <noreply at github.com>
date: 2017-08-06T19:32:39+02:00
summary:

[3.6] bpo-30808: Use _Py_atomic API for concurrency-sensitive signal state (GH-2417) (#3007)

* Improve signal delivery

Avoid using Py_AddPendingCall from signal handler, to avoid calling signal-unsafe functions.

* Remove unused function

* Improve comments

* Use _Py_atomic API for concurrency-sensitive signal state

* Add blurb
(cherry picked from commit 2c8a5e4c968217f9672340e520942c4ed788d8de)

files:
A Misc/NEWS.d/next/Core and Builtins/2017-07-17-12-12-59.bpo-30808.bA3zOv.rst
M Modules/signalmodule.c

diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-07-17-12-12-59.bpo-30808.bA3zOv.rst b/Misc/NEWS.d/next/Core and Builtins/2017-07-17-12-12-59.bpo-30808.bA3zOv.rst
new file mode 100644
index 00000000000..8adbbe7dc3a
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2017-07-17-12-12-59.bpo-30808.bA3zOv.rst	
@@ -0,0 +1 @@
+Use _Py_atomic API for concurrency-sensitive signal state.
diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c
index 30990f89c3e..0e6099fd361 100644
--- a/Modules/signalmodule.c
+++ b/Modules/signalmodule.c
@@ -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];
 
@@ -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;
@@ -240,11 +240,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
@@ -465,7 +467,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)
@@ -1264,11 +1266,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)
@@ -1492,7 +1494,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)
@@ -1513,7 +1515,7 @@ PyErr_CheckSignals(void)
     int i;
     PyObject *f;
 
-    if (!is_tripped)
+    if (!_Py_atomic_load(&is_tripped))
         return 0;
 
 #ifdef WITH_THREAD
@@ -1535,16 +1537,16 @@ 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,
@@ -1552,7 +1554,7 @@ PyErr_CheckSignals(void)
                 Py_DECREF(arglist);
             }
             if (!result) {
-                is_tripped = 1;
+                _Py_atomic_store(&is_tripped, 1);
                 return -1;
             }
 
@@ -1591,12 +1593,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;
@@ -1606,11 +1608,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);
     }
 }