mirror of https://github.com/python/cpython
bpo-43406: Fix possible race condition where ``PyErr_CheckSignals`` tries to execute a non-Python signal handler (GH-24756)
We can receive signals (at the C level, in `trip_signal()` in signalmodule.c) while `signal.signal` is being called to modify the corresponding handler. Later when `PyErr_CheckSignals()` is called to handle the given signal, the handler may be a non-callable object and would raise a cryptic asynchronous exception.
This commit is contained in:
parent
02ac6f41e5
commit
68245b7a10
|
@ -6,6 +6,7 @@ import socket
|
|||
import statistics
|
||||
import subprocess
|
||||
import sys
|
||||
import threading
|
||||
import time
|
||||
import unittest
|
||||
from test import support
|
||||
|
@ -1251,6 +1252,55 @@ class StressTest(unittest.TestCase):
|
|||
# Python handler
|
||||
self.assertEqual(len(sigs), N, "Some signals were lost")
|
||||
|
||||
@unittest.skipUnless(hasattr(signal, "SIGUSR1"),
|
||||
"test needs SIGUSR1")
|
||||
def test_stress_modifying_handlers(self):
|
||||
# bpo-43406: race condition between trip_signal() and signal.signal
|
||||
signum = signal.SIGUSR1
|
||||
num_sent_signals = 0
|
||||
num_received_signals = 0
|
||||
do_stop = False
|
||||
|
||||
def custom_handler(signum, frame):
|
||||
nonlocal num_received_signals
|
||||
num_received_signals += 1
|
||||
|
||||
def set_interrupts():
|
||||
nonlocal num_sent_signals
|
||||
while not do_stop:
|
||||
signal.raise_signal(signum)
|
||||
num_sent_signals += 1
|
||||
|
||||
def cycle_handlers():
|
||||
while num_sent_signals < 100:
|
||||
for i in range(20000):
|
||||
# Cycle between a Python-defined and a non-Python handler
|
||||
for handler in [custom_handler, signal.SIG_IGN]:
|
||||
signal.signal(signum, handler)
|
||||
|
||||
old_handler = signal.signal(signum, custom_handler)
|
||||
self.addCleanup(signal.signal, signum, old_handler)
|
||||
t = threading.Thread(target=set_interrupts)
|
||||
t.start()
|
||||
try:
|
||||
with support.catch_unraisable_exception() as cm:
|
||||
cycle_handlers()
|
||||
if cm.unraisable is not None:
|
||||
# An unraisable exception may be printed out when
|
||||
# a signal is ignored due to the aforementioned
|
||||
# race condition, check it.
|
||||
self.assertIsInstance(cm.unraisable.exc_value, OSError)
|
||||
self.assertIn(
|
||||
f"Signal {signum} ignored due to race condition",
|
||||
str(cm.unraisable.exc_value))
|
||||
# Sanity check that some signals were received, but not all
|
||||
self.assertGreater(num_received_signals, 0)
|
||||
self.assertLess(num_received_signals, num_sent_signals)
|
||||
finally:
|
||||
do_stop = True
|
||||
t.join()
|
||||
|
||||
|
||||
class RaiseSignalTest(unittest.TestCase):
|
||||
|
||||
def test_sigint(self):
|
||||
|
|
|
@ -0,0 +1,2 @@
|
|||
Fix a possible race condition where ``PyErr_CheckSignals`` tries to execute a
|
||||
non-Python signal handler.
|
|
@ -1706,10 +1706,34 @@ _PyErr_CheckSignalsTstate(PyThreadState *tstate)
|
|||
}
|
||||
_Py_atomic_store_relaxed(&Handlers[i].tripped, 0);
|
||||
|
||||
/* Signal handlers can be modified while a signal is received,
|
||||
* and therefore the fact that trip_signal() or PyErr_SetInterrupt()
|
||||
* was called doesn't guarantee that there is still a Python
|
||||
* signal handler for it by the time PyErr_CheckSignals() is called
|
||||
* (see bpo-43406).
|
||||
*/
|
||||
PyObject *func = Handlers[i].func;
|
||||
if (func == NULL || func == Py_None || func == IgnoreHandler ||
|
||||
func == DefaultHandler) {
|
||||
/* No Python signal handler due to aforementioned race condition.
|
||||
* We can't call raise() as it would break the assumption
|
||||
* that PyErr_SetInterrupt() only *simulates* an incoming
|
||||
* signal (i.e. it will never kill the process).
|
||||
* We also don't want to interrupt user code with a cryptic
|
||||
* asynchronous exception, so instead just write out an
|
||||
* unraisable error.
|
||||
*/
|
||||
PyErr_Format(PyExc_OSError,
|
||||
"Signal %i ignored due to race condition",
|
||||
i);
|
||||
PyErr_WriteUnraisable(Py_None);
|
||||
continue;
|
||||
}
|
||||
|
||||
PyObject *arglist = Py_BuildValue("(iO)", i, frame);
|
||||
PyObject *result;
|
||||
if (arglist) {
|
||||
result = _PyObject_Call(tstate, Handlers[i].func, arglist, NULL);
|
||||
result = _PyObject_Call(tstate, func, arglist, NULL);
|
||||
Py_DECREF(arglist);
|
||||
}
|
||||
else {
|
||||
|
|
Loading…
Reference in New Issue