From 2123fdc62a3c90cf0cfb29404c049fba55c136c1 Mon Sep 17 00:00:00 2001 From: jg Date: Tue, 3 Nov 2026 21:02:13 +0000 Subject: [PATCH 1/2] Make the signal handler table safe thread safe without a lock Utilise atomic_exchange and `QSBR` to ensure that accessing an old handler does not result in a potential UAF. --- Lib/test/test_signal.py | 37 +++++++++++++++++++ ...-11-02-22-45-00.gh-issue-116738.sGt4Hd.rst | 4 ++ Modules/signalmodule.c | 30 +++++++-------- 3 files changed, 56 insertions(+), 15 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-11-02-22-45-00.gh-issue-116738.sGt4Hd.rst diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py index 2cad18a69ff7c02..abf1ada6044e131 100644 --- a/Lib/test/test_signal.py +++ b/Lib/test/test_signal.py @@ -12,6 +12,7 @@ import threading import time import unittest +import weakref from test import support from test.support import ( force_not_colorized, is_apple, is_apple_mobile, os_helper, threading_helper @@ -1441,6 +1442,42 @@ def handler(a, b): signal.raise_signal(signal.SIGINT) self.assertTrue(is_ok) + def test_handler_replacing_itself_stays_alive(self): + events = [] + def handler(signum, frame): + events.append("enter") + signal.signal(signum, signal.SIG_DFL) + support.gc_collect() + events.append("exit") + handler = functools.partial(handler) + wr = weakref.ref(handler, lambda ref: events.append("freed")) + old_signal = signal.signal(signal.SIGINT, handler) + self.addCleanup(signal.signal, signal.SIGINT, old_signal) + del handler + + signal.raise_signal(signal.SIGINT) + support.gc_collect() + self.assertEqual(events, ["enter", "exit", "freed"]) + + def test_handler_replacing_itself_and_raising_is_freed(self): + events = [] + def handler(signum, frame): + events.append("enter") + signal.signal(signum, signal.SIG_DFL) + support.gc_collect() + events.append("exit") + raise ZeroDivisionError + handler = functools.partial(handler) + wr = weakref.ref(handler, lambda ref: events.append("freed")) + old_signal = signal.signal(signal.SIGINT, handler) + self.addCleanup(signal.signal, signal.SIGINT, old_signal) + del handler + + with self.assertRaises(ZeroDivisionError): + signal.raise_signal(signal.SIGINT) + support.gc_collect() + self.assertEqual(events, ["enter", "exit", "freed"]) + def test__thread_interrupt_main(self): # See https://github.com/python/cpython/issues/102397 code = """if 1: diff --git a/Misc/NEWS.d/next/Library/2026-11-02-22-45-00.gh-issue-116738.sGt4Hd.rst b/Misc/NEWS.d/next/Library/2026-11-02-22-45-00.gh-issue-116738.sGt4Hd.rst new file mode 100644 index 000000000000000..d486f9236d7f753 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-11-02-22-45-00.gh-issue-116738.sGt4Hd.rst @@ -0,0 +1,4 @@ +Fix a possible crash in :func:`signal.getsignal` on a free-threaded build when +another thread replaces the signal handler at the same time. A Python signal +handler that replaces itself with :func:`signal.signal` now stays alive until +it returns. diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index bc5aef55648e071..e959217ca3315ff 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -10,6 +10,7 @@ #include "pycore_fileutils.h" // _Py_BEGIN_SUPPRESS_IPH #include "pycore_interpframe.h" // _PyThreadState_GetFrame() #include "pycore_moduleobject.h" // _PyModule_GetState() +#include "pycore_object.h" // _PyObject_XDecRefDelayed() #include "pycore_pyerrors.h" // _PyErr_SetString() #include "pycore_pystate.h" // _PyThreadState_GET() #include "pycore_signal.h" // _Py_RestoreSignals() @@ -135,12 +136,14 @@ get_handler(int i) return (PyObject *)_Py_atomic_load_ptr(&Handlers[i].func); } -Py_LOCAL_INLINE(void) -set_handler(int i, PyObject* func) +Py_LOCAL_INLINE(PyObject *) +swap_handler(int i, PyObject *func) { - /* Store func with atomic operation to ensure - that PyErr_SetInterrupt is async-signal-safe. */ - _Py_atomic_store_ptr(&Handlers[i].func, func); + PyObject *old = _Py_atomic_exchange_ptr(&Handlers[i].func, + Py_XNewRef(func)); + PyObject *result = Py_XNewRef(old); + _PyObject_XDecRefDelayed(old); + return result; } @@ -532,9 +535,7 @@ signal_signal_impl(PyObject *module, int signalnum, PyObject *handler) return NULL; } - old_handler = get_handler(signalnum); - set_handler(signalnum, Py_NewRef(handler)); - + old_handler = swap_handler(signalnum, handler); if (old_handler != NULL) { return old_handler; } @@ -1595,9 +1596,7 @@ signal_get_set_handlers(signal_state_t *state, PyObject *mod_dict) } // If signal_module_exec() is called more than one, we must // clear the strong reference to the previous function. - PyObject* old_func = get_handler(signum); - set_handler(signum, Py_NewRef(func)); - Py_XDECREF(old_func); + Py_XDECREF(swap_handler(signum, func)); } // Install Python SIGINT handler which raises KeyboardInterrupt @@ -1609,8 +1608,8 @@ signal_get_set_handlers(signal_state_t *state, PyObject *mod_dict) return -1; } - set_handler(SIGINT, int_handler); - Py_DECREF(sigint_func); + Py_XDECREF(swap_handler(SIGINT, int_handler)); + Py_DECREF(int_handler); PyOS_setsig(SIGINT, signal_handler); } return 0; @@ -1744,9 +1743,8 @@ _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_int_relaxed(&Handlers[signum].tripped, 0); - set_handler(signum, NULL); + PyObject *func = swap_handler(signum, NULL); if (func != NULL && func != Py_None && !compare_handler(func, state->default_handler) @@ -1864,6 +1862,7 @@ _PyErr_CheckSignalsTstate(PyThreadState *tstate) "calling signal handler"); continue; } + Py_INCREF(func); PyObject *arglist = NULL; if (frame == NULL) { arglist = Py_BuildValue("(iO)", i, Py_None); @@ -1882,6 +1881,7 @@ _PyErr_CheckSignalsTstate(PyThreadState *tstate) else { result = NULL; } + Py_DECREF(func); if (!result) { /* On error, re-schedule a call to _PyErr_CheckSignalsTstate() */ _Py_atomic_store_int(&is_tripped, 1); From 8a42e247e44a738a602d413d211cf5fdcd2692c9 Mon Sep 17 00:00:00 2001 From: jg Date: Wed, 4 Nov 2026 22:53:26 +0000 Subject: [PATCH 2/2] Keep the async-signal-safety comment on the handler slot update --- Modules/signalmodule.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index e959217ca3315ff..d8d1261a88afef7 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -139,6 +139,8 @@ get_handler(int i) Py_LOCAL_INLINE(PyObject *) swap_handler(int i, PyObject *func) { + /* The slot is updated with an atomic operation so that + PyErr_SetInterrupt() stays async-signal-safe. */ PyObject *old = _Py_atomic_exchange_ptr(&Handlers[i].func, Py_XNewRef(func)); PyObject *result = Py_XNewRef(old);