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
4 changes: 4 additions & 0 deletions Include/internal/pycore_interp_structs.h
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,10 @@ struct _warnings_runtime_state {
PyObject *once_registry; /* Dict */
PyObject *default_action; /* String */
_PyRecursiveMutex lock;
#ifdef HAVE_FORK
// Whether the thread calling fork() owns the warnings lock.
bool lock_held_at_fork;
#endif
long filters_version;
PyObject *context;
};
Expand Down
3 changes: 3 additions & 0 deletions Include/internal/pycore_lock.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,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
95 changes: 95 additions & 0 deletions Lib/test/test_warnings/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from test.support import import_helper
from test.support import isolation
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 @@ -1632,6 +1633,100 @@ 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()
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 @@ -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()
Expand Down Expand Up @@ -716,6 +717,7 @@ PyOS_BeforeFork(void)
_PyImport_AcquireLock(interp);
_PyEval_StopTheWorldAll(&_PyRuntime);
HEAD_LOCK(&_PyRuntime);
_PyWarnings_BeforeFork(interp);
}

void
Expand Down Expand Up @@ -786,6 +788,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);
Expand Down
18 changes: 18 additions & 0 deletions Python/_warnings.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -276,6 +277,23 @@ warnings_lock_held(WarningsState *st)
return PyMutex_IsLocked(&st->lock.mutex);
}

#ifdef HAVE_FORK
void
_PyWarnings_BeforeFork(PyInterpreterState *interp)
{
WarningsState *st = warnings_get_state(interp);
st->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, st->lock_held_at_fork);
st->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 @@ -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)
Expand Down
Loading