Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Include/internal/pycore_lock.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,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
Expand Down
5 changes: 5 additions & 0 deletions Include/internal/pycore_warnings.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
100 changes: 100 additions & 0 deletions Lib/test/test_warnings/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1561,6 +1562,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):

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "pycore_signal.h" // Py_NSIG
#include "pycore_time.h" // _PyLong_FromTime_t()
#include "pycore_typeobject.h" // _PyType_AddMethod()
#include "pycore_warnings.h" // _PyWarnings_AfterFork()

#ifndef MS_WINDOWS
# include "posixmodule.h" // _PyLong_FromUid()
Expand Down Expand Up @@ -669,6 +670,7 @@ PyOS_BeforeFork(void)
_PyImport_AcquireLock(interp);
_PyEval_StopTheWorldAll(&_PyRuntime);
HEAD_LOCK(&_PyRuntime);
_PyWarnings_BeforeFork(interp);
}

void
Expand Down Expand Up @@ -732,6 +734,9 @@ PyOS_AfterFork_Child(void)
goto fatal_error;
}

// Thread-state cleanup can run finalizers that issue warnings.
_PyWarnings_AfterFork(tstate->interp);

reset_remotedebug_data(tstate);

reset_asyncio_state((_PyThreadStateImpl *)tstate);
Expand Down
23 changes: 23 additions & 0 deletions Python/_warnings.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "pycore_sysmodule.h" // _PySys_GetOptionalAttr()
#include "pycore_traceback.h" // _Py_DisplaySourceLine()
#include "pycore_unicodeobject.h" // _PyUnicode_EqualToASCIIString()
#include "pycore_warnings.h" // _PyWarnings_AfterFork()

#include <stdbool.h>
#include "clinic/_warnings.c.h"
Expand Down Expand Up @@ -264,6 +265,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)
{
Expand Down
19 changes: 19 additions & 0 deletions Python/lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,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)
Expand Down
1 change: 1 addition & 0 deletions Tools/c-analyzer/cpython/ignored.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,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/codecs.c - Py_hexdigits -
Expand Down
Loading