gh-81057: Move Signal-Related Globals to _PyRuntimeState (gh-100085)

#81057

Author: ericsnowcurrently

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;

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. */ \

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 <signal.h>                // NSIG
 
+
 #ifdef _SIG_MAXSIG
    // gh-91145: On FreeBSD, <signal.h> 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 <winsock2.h> 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 <windows.h> 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

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 <windows.h>
 #  ifdef HAVE_PROCESS_H
 #    include <process.h>
 #  endif
 #endif
 
+#include "pycore_signal.h"        // Py_NSIG
+
 #ifdef HAVE_SIGNAL_H
 #  include <signal.h>
 #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;

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) \

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	-
+# <none>
 
 
 ##################################