https://github.com/python/cpython/commit/81fe70e96630b1bfdaa4624b4f95fcaf6ba38025 commit: 81fe70e96630b1bfdaa4624b4f95fcaf6ba38025 branch: 3.15 author: Neil Schemenauer <[email protected]> committer: nascheme <[email protected]> date: 2026-10-19T14:54:15-07:00 summary:
[3.15] gh-158973: Reset the warnings lock after fork in the child (gh-158974) (gh-159614) Co-authored-by: Nathan Goldbaum <[email protected]> files: A Misc/NEWS.d/next/Core_and_Builtins/2026-10-07-00-00-00.gh-issue-158973.warnings-fork.rst M Include/internal/pycore_lock.h M Include/internal/pycore_warnings.h M Lib/test/test_warnings/__init__.py M Modules/posixmodule.c M Python/_warnings.c M Python/lock.c M Tools/c-analyzer/cpython/ignored.tsv diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h index e31d8b4e5c68c92..b86a3098b9af5d5 100644 --- a/Include/internal/pycore_lock.h +++ b/Include/internal/pycore_lock.h @@ -168,6 +168,9 @@ PyAPI_FUNC(void) _PyRecursiveMutex_Lock(_PyRecursiveMutex *m); extern PyLockStatus _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags flags); PyAPI_FUNC(void) _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m); extern int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m); +#ifdef HAVE_FORK +extern void _PyRecursiveMutex_at_fork_reinit(_PyRecursiveMutex *m, int owned); +#endif // A readers-writer (RW) lock. The lock supports multiple concurrent readers or // a single writer. The lock is write-preferring: if a writer is waiting while diff --git a/Include/internal/pycore_warnings.h b/Include/internal/pycore_warnings.h index e1348d529a22463..d3c0c549373257e 100644 --- a/Include/internal/pycore_warnings.h +++ b/Include/internal/pycore_warnings.h @@ -12,6 +12,11 @@ extern int _PyWarnings_InitState(PyInterpreterState *interp); extern PyObject* _PyWarnings_Init(void); +#ifdef HAVE_FORK +extern void _PyWarnings_BeforeFork(PyInterpreterState *interp); +extern void _PyWarnings_AfterFork(PyInterpreterState *interp); +#endif + extern void _PyErr_WarnUnawaitedCoroutine(PyObject *coro); extern void _PyErr_WarnUnawaitedAgenMethod(PyAsyncGenObject *agen, PyObject *method); diff --git a/Lib/test/test_warnings/__init__.py b/Lib/test/test_warnings/__init__.py index 992797d22e8cf27..c9aff12e596366f 100644 --- a/Lib/test/test_warnings/__init__.py +++ b/Lib/test/test_warnings/__init__.py @@ -14,6 +14,7 @@ from test import support from test.support import import_helper from test.support import os_helper +from test.support import threading_helper from test.support import warnings_helper from test.support import force_not_colorized from test.support.script_helper import assert_python_ok, assert_python_failure @@ -1621,6 +1622,105 @@ def test_release_lock_no_lock(self): ): c_warnings._release_lock() + @support.cpython_only + @unittest.skipUnless(support.has_fork_support, 'requires working os.fork') + @threading_helper.requires_working_threading() + def test_fork_other_thread_holds_lock(self): + code = textwrap.dedent(''' + import _warnings + import os + import threading + import warnings + from test import support + + parent_pid = os.getpid() + read_fd, write_fd = os.pipe() + local = threading.local() + ready = threading.Event() + release = threading.Event() + warnings.simplefilter('ignore', DeprecationWarning) + warnings.simplefilter('ignore', ResourceWarning) + + class WarnOnDelete: + def __del__(self): + if os.getpid() != parent_pid: + # Even an ignored warning must acquire the lock. + warnings.warn('child cleanup', ResourceWarning) + os.write(write_fd, b'finalized') + + def worker(): + local.obj = WarnOnDelete() + _warnings._acquire_lock() + _warnings._acquire_lock() + try: + ready.set() + release.wait() + finally: + _warnings._release_lock() + _warnings._release_lock() + + thread = threading.Thread(target=worker) + thread.start() + # Release the worker before fork's parent-side warning attempts + # to acquire the warnings lock. + os.register_at_fork(after_in_parent=release.set) + try: + assert ready.wait(support.LONG_TIMEOUT) + pid = os.fork() + if pid == 0: + # The finalizer must run before os.fork() returns. + os._exit(0) + support.wait_process(pid, exitcode=0) + finally: + release.set() + thread.join() + os.close(write_fd) + assert os.read(read_fd, 100) == b'finalized' + os.close(read_fd) + ''') + assert_python_ok('-c', code) + + @support.cpython_only + @unittest.skipUnless(support.has_fork_support, 'requires working os.fork') + def test_fork_current_thread_holds_lock(self): + code = textwrap.dedent(''' + import _warnings + import os + import warnings + from test import support + + warnings.simplefilter('ignore') + for _ in range(3): + _warnings._acquire_lock() + pid = os.fork() + if pid == 0: + # Ownership must also survive a second fork in the child. + child_pid = os.fork() + if child_pid != 0: + support.wait_process(child_pid, exitcode=0) + try: + # A warning must not change the inherited recursion depth. + warnings.warn('after fork') + for _ in range(3): + _warnings._release_lock() + try: + _warnings._release_lock() + except RuntimeError: + pass + else: + raise AssertionError('unexpected recursion depth') + _warnings._acquire_lock() + _warnings._release_lock() + except BaseException: + if pid == 0: + os._exit(1) + raise + if pid == 0: + os._exit(0) + support.wait_process(pid, exitcode=0) + ''') + assert_python_ok('-c', code) + class _DeprecatedTest(BaseTest, unittest.TestCase): diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-07-00-00-00.gh-issue-158973.warnings-fork.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-07-00-00-00.gh-issue-158973.warnings-fork.rst new file mode 100644 index 000000000000000..9a98cf6e0333845 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-07-00-00-00.gh-issue-158973.warnings-fork.rst @@ -0,0 +1,3 @@ +Reset the warnings lock in the child after :func:`os.fork`, before clearing +other threads' states, to avoid deadlocks when their finalizers issue warnings. +Preserve the lock's recursion depth if the forking thread owns it. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 660d3da252985c1..1a6d69824786560 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -31,6 +31,7 @@ #include "pycore_time.h" // _PyLong_FromTime_t() #include "pycore_tuple.h" // _PyTuple_FromPairSteal #include "pycore_typeobject.h" // _PyType_AddMethod() +#include "pycore_warnings.h" // _PyWarnings_AfterFork() #ifndef MS_WINDOWS # include "posixmodule.h" // _PyLong_FromUid() @@ -709,6 +710,7 @@ PyOS_BeforeFork(void) _PyImport_AcquireLock(interp); _PyEval_StopTheWorldAll(&_PyRuntime); HEAD_LOCK(&_PyRuntime); + _PyWarnings_BeforeFork(interp); } void @@ -779,6 +781,9 @@ PyOS_AfterFork_Child(void) _Py_jit_debug_mutex = (PyMutex){0}; #endif + // Thread-state cleanup can run finalizers that issue warnings. + _PyWarnings_AfterFork(tstate->interp); + reset_remotedebug_data(tstate); reset_asyncio_state((_PyThreadStateImpl *)tstate); diff --git a/Python/_warnings.c b/Python/_warnings.c index 586cec195fac680..dd48f740d261764 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -9,6 +9,7 @@ #include "pycore_traceback.h" // _Py_DisplaySourceLine() #include "pycore_tuple.h" // _PyTuple_FromPair #include "pycore_unicodeobject.h" // _PyUnicode_EqualToASCIIString() +#include "pycore_warnings.h" // _PyWarnings_AfterFork() #include <stdbool.h> #include "clinic/_warnings.c.h" @@ -276,6 +277,28 @@ warnings_lock_held(WarningsState *st) return PyMutex_IsLocked(&st->lock.mutex); } +#ifdef HAVE_FORK +// PyOS_BeforeFork() holds HEAD_LOCK while saving this value and until fork(), +// so concurrent forks cannot overwrite it before the child inherits it. +// The child consumes its private copy before running Python callbacks. +static bool warnings_lock_held_at_fork; + +void +_PyWarnings_BeforeFork(PyInterpreterState *interp) +{ + WarningsState *st = warnings_get_state(interp); + warnings_lock_held_at_fork = _PyRecursiveMutex_IsLockedByCurrentThread(&st->lock); +} + +void +_PyWarnings_AfterFork(PyInterpreterState *interp) +{ + WarningsState *st = warnings_get_state(interp); + _PyRecursiveMutex_at_fork_reinit(&st->lock, warnings_lock_held_at_fork); + warnings_lock_held_at_fork = false; +} +#endif + static PyObject * get_warnings_context(PyInterpreterState *interp) { diff --git a/Python/lock.c b/Python/lock.c index 84f18a0c15a769c..001cf0b1dbe9eee 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -453,6 +453,25 @@ _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m) return 0; } +#ifdef HAVE_FORK +// Reinitialize the mutex in the child process after fork(). The caller must +// record before fork() whether the forking thread owns the mutex, since the +// thread ident can change in the child (gh-126688). If it does, the child +// keeps the mutex with the same recursion level; otherwise the owner and any +// waiters no longer exist in the child and the mutex is reset. +void +_PyRecursiveMutex_at_fork_reinit(_PyRecursiveMutex *m, int owned) +{ + if (owned) { + m->mutex = (PyMutex){._bits = _Py_LOCKED}; + _Py_atomic_store_ullong_relaxed(&m->thread, PyThread_get_thread_ident_ex()); + } + else { + memset(m, 0, sizeof(*m)); + } +} +#endif + #define _Py_WRITE_LOCKED 1 #define _PyRWMutex_READER_SHIFT 2 #define _Py_RWMUTEX_MAX_READERS (UINTPTR_MAX >> _PyRWMutex_READER_SHIFT) diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index aedc26a3297c64e..6024eb7e093eacf 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -362,6 +362,7 @@ Objects/unicodeobject.c unicode_translate_call_errorhandler argparse - Parser/parser.c - reserved_keywords - Parser/parser.c - soft_keywords - Parser/lexer/lexer.c - type_comment_prefix - +Python/_warnings.c - warnings_lock_held_at_fork - Python/ceval.c - _PyEval_BinaryOps - Python/ceval.c - _Py_INTERPRETER_TRAMPOLINE_INSTRUCTIONS - Python/ceval.c - _Py_INTERPRETER_TRAMPOLINE_INSTRUCTIONS_PTR - _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3//lists/python-checkins.python.org Member address: [email protected]
