From 5eb28bca9fc963607189e3b3e1403f341dbf640a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 12 Dec 2022 16:50:19 -0700 Subject: [PATCH] gh-81057: Move Signal-Related Globals to _PyRuntimeState (gh-100085) https://github.com/python/cpython/issues/81057 --- Include/internal/pycore_runtime.h | 7 +- Include/internal/pycore_runtime_init.h | 1 + Include/internal/pycore_signal.h | 63 ++++++++++++++++++ Modules/signalmodule.c | 71 +++++---------------- Python/pylifecycle.c | 1 - Tools/c-analyzer/cpython/globals-to-fix.tsv | 8 +-- 6 files changed, 82 insertions(+), 69 deletions(-) diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index b9ed8f59315..92ed45956c9 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -23,6 +23,7 @@ extern "C" { #include "pycore_pyhash.h" // struct pyhash_runtime_state #include "pycore_pythread.h" // struct _pythread_runtime_state #include "pycore_obmalloc.h" // struct obmalloc_state +#include "pycore_signal.h" // struct _signals_runtime_state #include "pycore_time.h" // struct _time_runtime_state #include "pycore_tracemalloc.h" // struct _tracemalloc_runtime_state #include "pycore_unicodeobject.h" // struct _Py_unicode_runtime_ids @@ -93,13 +94,9 @@ typedef struct pyruntimestate { struct _pymem_allocators allocators; struct _obmalloc_state obmalloc; struct pyhash_runtime_state pyhash_state; - struct { - /* True if the main interpreter thread exited due to an unhandled - * KeyboardInterrupt exception, suggesting the user pressed ^C. */ - int unhandled_keyboard_interrupt; - } signals; struct _time_runtime_state time; struct _pythread_runtime_state threads; + struct _signals_runtime_state signals; struct pyinterpreters { PyThread_type_lock mutex; diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h index 9677a727c44..1431096e2d2 100644 --- a/Include/internal/pycore_runtime_init.h +++ b/Include/internal/pycore_runtime_init.h @@ -26,6 +26,7 @@ extern "C" { }, \ .obmalloc = _obmalloc_state_INIT(runtime.obmalloc), \ .pyhash_state = pyhash_state_INIT, \ + .signals = _signals_RUNTIME_INIT, \ .interpreters = { \ /* This prevents interpreters from getting created \ until _PyInterpreterState_Enable() is called. */ \ diff --git a/Include/internal/pycore_signal.h b/Include/internal/pycore_signal.h index b921dd170e9..ca3f69d09fc 100644 --- a/Include/internal/pycore_signal.h +++ b/Include/internal/pycore_signal.h @@ -10,8 +10,11 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif +#include "pycore_atomic.h" // _Py_atomic_address + #include // NSIG + #ifdef _SIG_MAXSIG // gh-91145: On FreeBSD, defines NSIG as 32: it doesn't include // realtime signals: [SIGRTMIN,SIGRTMAX]. Use _SIG_MAXSIG instead. For @@ -29,6 +32,66 @@ extern "C" { # define Py_NSIG 64 // Use a reasonable default value #endif +#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; + } handlers[Py_NSIG]; + + volatile struct { +#ifdef MS_WINDOWS + /* This would be "SOCKET fd" if were always included. + It isn't so we must cast to SOCKET where appropriate. */ + volatile int fd; +#elif defined(__VXWORKS__) + int fd; +#else + sig_atomic_t fd; +#endif + + int warn_on_full_buffer; +#ifdef MS_WINDOWS + int use_send; +#endif + } wakeup; + + /* Speed up sigcheck() when none tripped */ + _Py_atomic_int is_tripped; + + /* These objects necessarily belong to the main interpreter. */ + PyObject *default_handler; + PyObject *ignore_handler; + +#ifdef MS_WINDOWS + /* This would be "HANDLE sigint_event" if were always included. + It isn't so we must cast to HANDLE everywhere "sigint_event" is used. */ + void *sigint_event; +#endif + + /* True if the main interpreter thread exited due to an unhandled + * KeyboardInterrupt exception, suggesting the user pressed ^C. */ + int unhandled_keyboard_interrupt; +}; + +#ifdef MS_WINDOWS +# define _signals_WAKEUP_INIT \ + {.fd = INVALID_FD, .warn_on_full_buffer = 1, .use_send = 0} +#else +# define _signals_WAKEUP_INIT \ + {.fd = INVALID_FD, .warn_on_full_buffer = 1} +#endif + +#define _signals_RUNTIME_INIT \ + { \ + .wakeup = _signals_WAKEUP_INIT, \ + } + + #ifdef __cplusplus } #endif diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index c539787e582..538a7e85bc9 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -13,7 +13,7 @@ #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_pyerrors.h" // _PyErr_SetString() #include "pycore_pystate.h" // _PyThreadState_GET() -#include "pycore_signal.h" // Py_NSIG +#include "pycore_signal.h" #ifndef MS_WINDOWS # include "posixmodule.h" @@ -23,12 +23,13 @@ #endif #ifdef MS_WINDOWS -# include # ifdef HAVE_PROCESS_H # include # endif #endif +#include "pycore_signal.h" // Py_NSIG + #ifdef HAVE_SIGNAL_H # include #endif @@ -100,47 +101,13 @@ class sigset_t_converter(CConverter): may not be the thread that received the signal. */ -static 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; -} Handlers[Py_NSIG]; - -#ifdef MS_WINDOWS -#define INVALID_FD ((SOCKET_T)-1) - -static volatile struct { - SOCKET_T fd; - int warn_on_full_buffer; - int use_send; -} wakeup = {.fd = INVALID_FD, .warn_on_full_buffer = 1, .use_send = 0}; -#else -#define INVALID_FD (-1) -static volatile struct { -#ifdef __VXWORKS__ - int fd; -#else - sig_atomic_t fd; -#endif - int warn_on_full_buffer; -} wakeup = {.fd = INVALID_FD, .warn_on_full_buffer = 1}; -#endif - -/* Speed up sigcheck() when none tripped */ -static _Py_atomic_int is_tripped; - -typedef struct { - PyObject *default_handler; - PyObject *ignore_handler; -#ifdef MS_WINDOWS - HANDLE sigint_event; -#endif -} signal_state_t; +#define Handlers _PyRuntime.signals.handlers +#define wakeup _PyRuntime.signals.wakeup +#define is_tripped _PyRuntime.signals.is_tripped // State shared by all Python interpreters -static signal_state_t signal_global_state = {0}; +typedef struct _signals_runtime_state signal_state_t; +#define signal_global_state _PyRuntime.signals #if defined(HAVE_GETITIMER) || defined(HAVE_SETITIMER) # define PYHAVE_ITIMER_ERROR @@ -331,13 +298,7 @@ trip_signal(int sig_num) See bpo-30038 for more details. */ - int fd; -#ifdef MS_WINDOWS - fd = Py_SAFE_DOWNCAST(wakeup.fd, SOCKET_T, int); -#else - fd = wakeup.fd; -#endif - + int fd = wakeup.fd; if (fd != INVALID_FD) { unsigned char byte = (unsigned char)sig_num; #ifdef MS_WINDOWS @@ -407,7 +368,7 @@ signal_handler(int sig_num) #ifdef MS_WINDOWS if (sig_num == SIGINT) { signal_state_t *state = &signal_global_state; - SetEvent(state->sigint_event); + SetEvent((HANDLE)state->sigint_event); } #endif } @@ -822,7 +783,7 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args, PyObject *kwds) } old_sockfd = wakeup.fd; - wakeup.fd = sockfd; + wakeup.fd = Py_SAFE_DOWNCAST(sockfd, SOCKET_T, int); wakeup.warn_on_full_buffer = warn_on_full_buffer; wakeup.use_send = is_socket; @@ -873,11 +834,7 @@ PySignal_SetWakeupFd(int fd) fd = -1; } -#ifdef MS_WINDOWS - int old_fd = Py_SAFE_DOWNCAST(wakeup.fd, SOCKET_T, int); -#else int old_fd = wakeup.fd; -#endif wakeup.fd = fd; wakeup.warn_on_full_buffer = 1; return old_fd; @@ -1654,6 +1611,8 @@ signal_module_exec(PyObject *m) signal_state_t *state = &signal_global_state; _signal_module_state *modstate = get_signal_state(m); + // XXX For proper isolation, these values must be guaranteed + // to be effectively const (e.g. immortal). modstate->default_handler = state->default_handler; // borrowed ref modstate->ignore_handler = state->ignore_handler; // borrowed ref @@ -1783,7 +1742,7 @@ _PySignal_Fini(void) #ifdef MS_WINDOWS if (state->sigint_event != NULL) { - CloseHandle(state->sigint_event); + CloseHandle((HANDLE)state->sigint_event); state->sigint_event = NULL; } #endif @@ -2009,7 +1968,7 @@ _PySignal_Init(int install_signal_handlers) #ifdef MS_WINDOWS /* Create manual-reset event, initially unset */ - state->sigint_event = CreateEvent(NULL, TRUE, FALSE, FALSE); + state->sigint_event = (void *)CreateEvent(NULL, TRUE, FALSE, FALSE); if (state->sigint_event == NULL) { PyErr_SetFromWindowsErr(0); return -1; diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index e13660f5113..1cb0e4d747e 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -54,7 +54,6 @@ extern void _PyIO_Fini(void); #ifdef MS_WINDOWS # undef BYTE -# include "windows.h" extern PyTypeObject PyWindowsConsoleIO_Type; # define PyWindowsConsoleIO_Check(op) \ diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index eb57f95abd1..fcec95bbb2f 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -365,13 +365,7 @@ Modules/itertoolsmodule.c - ziplongest_type - ################################## ## global non-objects to fix in builtin modules -##----------------------- -## state - -Modules/signalmodule.c - is_tripped - -Modules/signalmodule.c - signal_global_state - -Modules/signalmodule.c - wakeup - -Modules/signalmodule.c - Handlers - +# ##################################