From 67e8d416cc5e14c181e4f883ffb4d87f2a647ae1 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 10 Oct 2023 08:26:29 +0900 Subject: [PATCH] gh-109693: Use pyatomic.h for signal module (gh-110480) --- Include/internal/pycore_signal.h | 16 +++++++-------- Modules/signalmodule.c | 35 ++++++++++++++++---------------- Python/ceval_gil.c | 2 +- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/Include/internal/pycore_signal.h b/Include/internal/pycore_signal.h index b38b1d30112b60..47213a34ab77b5 100644 --- a/Include/internal/pycore_signal.h +++ b/Include/internal/pycore_signal.h @@ -10,7 +10,6 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif -#include "pycore_atomic.h" // _Py_atomic_address #include // NSIG @@ -38,12 +37,10 @@ PyAPI_FUNC(void) _Py_RestoreSignals(void); #define INVALID_FD (-1) struct _signals_runtime_state { - volatile struct { - _Py_atomic_int tripped; - /* func is atomic to ensure that PyErr_SetInterrupt is async-signal-safe - * (even though it would probably be otherwise, anyway). - */ - _Py_atomic_address func; + struct { + // tripped and func should be accessed using atomic ops. + int tripped; + PyObject* func; } handlers[Py_NSIG]; volatile struct { @@ -63,8 +60,9 @@ struct _signals_runtime_state { #endif } wakeup; - /* Speed up sigcheck() when none tripped */ - _Py_atomic_int is_tripped; + /* Speed up sigcheck() when none tripped. + is_tripped should be accessed using atomic ops. */ + int is_tripped; /* These objects necessarily belong to the main interpreter. */ PyObject *default_handler; diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index a3c004f2a57d30..bc5cdf0ab6b5f2 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -4,7 +4,6 @@ /* XXX Signals should be recorded per thread, now we have thread state. */ #include "Python.h" -#include "pycore_atomic.h" // _Py_atomic_int #include "pycore_call.h" // _PyObject_Call() #include "pycore_ceval.h" // _PyEval_SignalReceived() #include "pycore_emscripten_signal.h" // _Py_CHECK_EMSCRIPTEN_SIGNALS @@ -124,13 +123,15 @@ typedef struct { Py_LOCAL_INLINE(PyObject *) get_handler(int i) { - return (PyObject *)_Py_atomic_load(&Handlers[i].func); + return (PyObject *)_Py_atomic_load_ptr(&Handlers[i].func); } Py_LOCAL_INLINE(void) set_handler(int i, PyObject* func) { - _Py_atomic_store(&Handlers[i].func, (uintptr_t)func); + /* Store func with atomic operation to ensure + that PyErr_SetInterrupt is async-signal-safe. */ + _Py_atomic_store_ptr(&Handlers[i].func, func); } @@ -267,11 +268,11 @@ report_wakeup_send_error(void* data) static void trip_signal(int sig_num) { - _Py_atomic_store_relaxed(&Handlers[sig_num].tripped, 1); + _Py_atomic_store_int(&Handlers[sig_num].tripped, 1); /* Set is_tripped after setting .tripped, as it gets cleared in PyErr_CheckSignals() before .tripped. */ - _Py_atomic_store(&is_tripped, 1); + _Py_atomic_store_int(&is_tripped, 1); /* Signals are always handled by the main interpreter */ PyInterpreterState *interp = _PyInterpreterState_Main(); @@ -1731,7 +1732,7 @@ _PySignal_Fini(void) // Restore default signals and clear handlers for (int signum = 1; signum < Py_NSIG; signum++) { PyObject *func = get_handler(signum); - _Py_atomic_store_relaxed(&Handlers[signum].tripped, 0); + _Py_atomic_store_int_relaxed(&Handlers[signum].tripped, 0); set_handler(signum, NULL); if (func != NULL && func != Py_None @@ -1785,7 +1786,7 @@ int _PyErr_CheckSignalsTstate(PyThreadState *tstate) { _Py_CHECK_EMSCRIPTEN_SIGNALS(); - if (!_Py_atomic_load(&is_tripped)) { + if (!_Py_atomic_load_int(&is_tripped)) { return 0; } @@ -1803,15 +1804,15 @@ _PyErr_CheckSignalsTstate(PyThreadState *tstate) * we receive a signal i after we zero is_tripped and before we * check Handlers[i].tripped. */ - _Py_atomic_store(&is_tripped, 0); + _Py_atomic_store_int(&is_tripped, 0); _PyInterpreterFrame *frame = _PyThreadState_GetFrame(tstate); signal_state_t *state = &signal_global_state; for (int i = 1; i < Py_NSIG; i++) { - if (!_Py_atomic_load_relaxed(&Handlers[i].tripped)) { + if (!_Py_atomic_load_int_relaxed(&Handlers[i].tripped)) { continue; } - _Py_atomic_store_relaxed(&Handlers[i].tripped, 0); + _Py_atomic_store_int_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() @@ -1857,7 +1858,7 @@ _PyErr_CheckSignalsTstate(PyThreadState *tstate) } if (!result) { /* On error, re-schedule a call to _PyErr_CheckSignalsTstate() */ - _Py_atomic_store(&is_tripped, 1); + _Py_atomic_store_int(&is_tripped, 1); return -1; } @@ -1975,7 +1976,7 @@ _PySignal_Init(int install_signal_handlers) #endif for (int signum = 1; signum < Py_NSIG; signum++) { - _Py_atomic_store_relaxed(&Handlers[signum].tripped, 0); + _Py_atomic_store_int_relaxed(&Handlers[signum].tripped, 0); } if (install_signal_handlers) { @@ -1997,11 +1998,11 @@ _PyOS_InterruptOccurred(PyThreadState *tstate) return 0; } - if (!_Py_atomic_load_relaxed(&Handlers[SIGINT].tripped)) { + if (!_Py_atomic_load_int_relaxed(&Handlers[SIGINT].tripped)) { return 0; } - _Py_atomic_store_relaxed(&Handlers[SIGINT].tripped, 0); + _Py_atomic_store_int_relaxed(&Handlers[SIGINT].tripped, 0); return 1; } @@ -2019,13 +2020,13 @@ PyOS_InterruptOccurred(void) static void _clear_pending_signals(void) { - if (!_Py_atomic_load(&is_tripped)) { + if (!_Py_atomic_load_int(&is_tripped)) { return; } - _Py_atomic_store(&is_tripped, 0); + _Py_atomic_store_int(&is_tripped, 0); for (int i = 1; i < Py_NSIG; ++i) { - _Py_atomic_store_relaxed(&Handlers[i].tripped, 0); + _Py_atomic_store_int_relaxed(&Handlers[i].tripped, 0); } } diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index e269efa9e2af80..78f0217e5fe821 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -76,7 +76,7 @@ update_eval_breaker_from_thread(PyInterpreterState *interp, PyThreadState *tstat _Py_set_eval_breaker_bit(interp, _PY_CALLS_TO_DO_BIT, 1); } if (_Py_ThreadCanHandleSignals(interp)) { - if (_Py_atomic_load(&_PyRuntime.signals.is_tripped)) { + if (_Py_atomic_load_int(&_PyRuntime.signals.is_tripped)) { _Py_set_eval_breaker_bit(interp, _PY_SIGNALS_PENDING_BIT, 1); } }