bpo-42296: On Windows, fix CTRL+C regression (GH-23257)

miss-islington · vstinner · web-flow · commit e5729aef6ff6 · 2020-11-13T06:11:38.000-08:00
On Windows, fix a regression in signal handling which prevented to interrupt a program using CTRL+C. The signal handler can be run in a thread different than the Python thread, in which case the test deciding if the thread can handle signals is wrong. On Windows, _PyEval_SignalReceived() now always sets eval_breaker to 1 since it cannot test _Py_ThreadCanHandleSignals(), and eval_frame_handle_pending() always calls _Py_ThreadCanHandleSignals() to recompute eval_breaker. (cherry picked from commit d96a7a8) Co-authored-by: Victor Stinner <vstinner@python.org>
diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-11-13-13-53-11.bpo-42296.DuGrLJ.rst b/Misc/NEWS.d/next/Core and Builtins/2020-11-13-13-53-11.bpo-42296.DuGrLJ.rst
@@ -0,0 +1,4 @@
+On Windows, fix a regression in signal handling which prevented to interrupt
+a program using CTRL+C. The signal handler can be run in a thread different
+than the Python thread, in which case the test deciding if the thread can
+handle signals is wrong.
diff --git a/Python/ceval.c b/Python/ceval.c
@@ -196,13 +196,18 @@ UNSIGNAL_PENDING_CALLS(PyInterpreterState *interp)
 
 
 static inline void
-SIGNAL_PENDING_SIGNALS(PyInterpreterState *interp)
+SIGNAL_PENDING_SIGNALS(PyInterpreterState *interp, int force)
 {
     struct _ceval_runtime_state *ceval = &interp->runtime->ceval;
     struct _ceval_state *ceval2 = &interp->ceval;
     _Py_atomic_store_relaxed(&ceval->signals_pending, 1);
-    /* eval_breaker is not set to 1 if thread_can_handle_signals() is false */
-    COMPUTE_EVAL_BREAKER(interp, ceval, ceval2);
+    if (force) {
+        _Py_atomic_store_relaxed(&ceval2->eval_breaker, 1);
+    }
+    else {
+        /* eval_breaker is not set to 1 if thread_can_handle_signals() is false */
+        COMPUTE_EVAL_BREAKER(interp, ceval, ceval2);
+    }
 }
 
 
@@ -491,10 +496,22 @@ PyEval_RestoreThread(PyThreadState *tstate)
 void
 _PyEval_SignalReceived(PyInterpreterState *interp)
 {
+#ifdef MS_WINDOWS
+    // bpo-42296: On Windows, _PyEval_SignalReceived() is called from a signal
+    // handler which can run in a thread different than the Python thread, in
+    // which case _Py_ThreadCanHandleSignals() is wrong. Ignore
+    // _Py_ThreadCanHandleSignals() and always set eval_breaker to 1.
+    //
+    // The next eval_frame_handle_pending() call will call
+    // _Py_ThreadCanHandleSignals() to recompute eval_breaker.
+    int force = 1;
+#else
+    int force = 0;
+#endif
     /* bpo-30703: Function called when the C signal handler of Python gets a
        signal. We cannot queue a callback using _PyEval_AddPendingCall() since
        that function is not async-signal-safe. */
-    SIGNAL_PENDING_SIGNALS(interp);
+    SIGNAL_PENDING_SIGNALS(interp, force);
 }
 
 /* Push one item onto the queue while holding the lock. */
@@ -594,7 +611,7 @@ handle_signals(PyThreadState *tstate)
     UNSIGNAL_PENDING_SIGNALS(tstate->interp);
     if (_PyErr_CheckSignalsTstate(tstate) < 0) {
         /* On failure, re-schedule a call to handle_signals(). */
-        SIGNAL_PENDING_SIGNALS(tstate->interp);
+        SIGNAL_PENDING_SIGNALS(tstate->interp, 0);
         return -1;
     }
     return 0;
@@ -883,6 +900,17 @@ eval_frame_handle_pending(PyThreadState *tstate)
         return -1;
     }
 
+#ifdef MS_WINDOWS
+    // bpo-42296: On Windows, _PyEval_SignalReceived() can be called in a
+    // different thread than the Python thread, in which case
+    // _Py_ThreadCanHandleSignals() is wrong. Recompute eval_breaker in the
+    // current Python thread with the correct _Py_ThreadCanHandleSignals()
+    // value. It prevents to interrupt the eval loop at every instruction if
+    // the current Python thread cannot handle signals (if
+    // _Py_ThreadCanHandleSignals() is false).
+    COMPUTE_EVAL_BREAKER(tstate->interp, ceval, ceval2);
+#endif
+
     return 0;
 }
 
