Skip to content

Commit 9dc342a

Browse files
naschememiss-islington
authored andcommitted
gh-158973: Reset the warnings lock after fork in the child (gh-158974)
(cherry picked from commit 8013779) Co-authored-by: Neil Schemenauer <nas-github@arctrix.com>
1 parent a73827e commit 9dc342a

8 files changed

Lines changed: 152 additions & 0 deletions

File tree

‎Include/internal/pycore_interp_structs.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,10 @@ struct _warnings_runtime_state {
661661
PyObject *once_registry; /* Dict */
662662
PyObject *default_action; /* String */
663663
_PyRecursiveMutex lock;
664+
#ifdef HAVE_FORK
665+
// Whether the thread calling fork() owns the warnings lock.
666+
bool lock_held_at_fork;
667+
#endif
664668
long filters_version;
665669
PyObject *context;
666670
};

‎Include/internal/pycore_lock.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,9 @@ PyAPI_FUNC(void) _PyRecursiveMutex_Lock(_PyRecursiveMutex *m);
168168
extern PyLockStatus _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags flags);
169169
PyAPI_FUNC(void) _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m);
170170
extern int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m);
171+
#ifdef HAVE_FORK
172+
extern void _PyRecursiveMutex_at_fork_reinit(_PyRecursiveMutex *m, int owned);
173+
#endif
171174

172175
// A readers-writer (RW) lock. The lock supports multiple concurrent readers or
173176
// a single writer. The lock is write-preferring: if a writer is waiting while

‎Include/internal/pycore_warnings.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ extern int _PyWarnings_InitState(PyInterpreterState *interp);
1212

1313
extern PyObject* _PyWarnings_Init(void);
1414

15+
#ifdef HAVE_FORK
16+
extern void _PyWarnings_BeforeFork(PyInterpreterState *interp);
17+
extern void _PyWarnings_AfterFork(PyInterpreterState *interp);
18+
#endif
19+
1520
extern void _PyErr_WarnUnawaitedCoroutine(PyObject *coro);
1621
extern void _PyErr_WarnUnawaitedAgenMethod(PyAsyncGenObject *agen, PyObject *method);
1722

‎Lib/test/test_warnings/__init__.py‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from test import support
1515
from test.support import import_helper
1616
from test.support import os_helper
17+
from test.support import threading_helper
1718
from test.support import warnings_helper
1819
from test.support import force_not_colorized
1920
from test.support.script_helper import assert_python_ok, assert_python_failure
@@ -1621,6 +1622,100 @@ def test_release_lock_no_lock(self):
16211622
):
16221623
c_warnings._release_lock()
16231624

1625+
@support.cpython_only
1626+
@unittest.skipUnless(support.has_fork_support, 'requires working os.fork')
1627+
@threading_helper.requires_working_threading()
1628+
def test_fork_other_thread_holds_lock(self):
1629+
code = textwrap.dedent('''
1630+
import _warnings
1631+
import os
1632+
import threading
1633+
import warnings
1634+
from test import support
1635+
1636+
parent_pid = os.getpid()
1637+
read_fd, write_fd = os.pipe()
1638+
local = threading.local()
1639+
ready = threading.Event()
1640+
release = threading.Event()
1641+
warnings.simplefilter('ignore', DeprecationWarning)
1642+
warnings.simplefilter('ignore', ResourceWarning)
1643+
1644+
class WarnOnDelete:
1645+
def __del__(self):
1646+
if os.getpid() != parent_pid:
1647+
# Even an ignored warning must acquire the lock.
1648+
warnings.warn('child cleanup', ResourceWarning)
1649+
os.write(write_fd, b'finalized')
1650+
1651+
def worker():
1652+
local.obj = WarnOnDelete()
1653+
_warnings._acquire_lock()
1654+
_warnings._acquire_lock()
1655+
try:
1656+
ready.set()
1657+
release.wait()
1658+
finally:
1659+
_warnings._release_lock()
1660+
_warnings._release_lock()
1661+
1662+
thread = threading.Thread(target=worker)
1663+
thread.start()
1664+
# Release the worker before fork's parent-side warning attempts
1665+
# to acquire the warnings lock.
1666+
os.register_at_fork(after_in_parent=release.set)
1667+
try:
1668+
assert ready.wait(support.LONG_TIMEOUT)
1669+
pid = os.fork()
1670+
if pid == 0:
1671+
# The finalizer must run before os.fork() returns.
1672+
os._exit(0)
1673+
support.wait_process(pid, exitcode=0)
1674+
finally:
1675+
release.set()
1676+
thread.join()
1677+
os.close(write_fd)
1678+
assert os.read(read_fd, 100) == b'finalized'
1679+
os.close(read_fd)
1680+
''')
1681+
assert_python_ok('-c', code)
1682+
1683+
@support.cpython_only
1684+
@unittest.skipUnless(support.has_fork_support, 'requires working os.fork')
1685+
def test_fork_current_thread_holds_lock(self):
1686+
code = textwrap.dedent('''
1687+
import _warnings
1688+
import os
1689+
import warnings
1690+
from test import support
1691+
1692+
warnings.simplefilter('ignore')
1693+
for _ in range(3):
1694+
_warnings._acquire_lock()
1695+
pid = os.fork()
1696+
try:
1697+
# A warning must not change the inherited recursion depth.
1698+
warnings.warn('after fork')
1699+
for _ in range(3):
1700+
_warnings._release_lock()
1701+
try:
1702+
_warnings._release_lock()
1703+
except RuntimeError:
1704+
pass
1705+
else:
1706+
raise AssertionError('unexpected recursion depth')
1707+
_warnings._acquire_lock()
1708+
_warnings._release_lock()
1709+
except BaseException:
1710+
if pid == 0:
1711+
os._exit(1)
1712+
raise
1713+
if pid == 0:
1714+
os._exit(0)
1715+
support.wait_process(pid, exitcode=0)
1716+
''')
1717+
assert_python_ok('-c', code)
1718+
16241719

16251720
class _DeprecatedTest(BaseTest, unittest.TestCase):
16261721

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Reset the warnings lock in the child after :func:`os.fork`, before clearing
2+
other threads' states, to avoid deadlocks when their finalizers issue warnings.
3+
Preserve the lock's recursion depth if the forking thread owns it.

‎Modules/posixmodule.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "pycore_time.h" // _PyLong_FromTime_t()
3232
#include "pycore_tuple.h" // _PyTuple_FromPairSteal
3333
#include "pycore_typeobject.h" // _PyType_AddMethod()
34+
#include "pycore_warnings.h" // _PyWarnings_AfterFork()
3435

3536
#ifndef MS_WINDOWS
3637
# include "posixmodule.h" // _PyLong_FromUid()
@@ -709,6 +710,7 @@ PyOS_BeforeFork(void)
709710
_PyImport_AcquireLock(interp);
710711
_PyEval_StopTheWorldAll(&_PyRuntime);
711712
HEAD_LOCK(&_PyRuntime);
713+
_PyWarnings_BeforeFork(interp);
712714
}
713715

714716
void
@@ -779,6 +781,9 @@ PyOS_AfterFork_Child(void)
779781
_Py_jit_debug_mutex = (PyMutex){0};
780782
#endif
781783

784+
// Thread-state cleanup can run finalizers that issue warnings.
785+
_PyWarnings_AfterFork(tstate->interp);
786+
782787
reset_remotedebug_data(tstate);
783788

784789
reset_asyncio_state((_PyThreadStateImpl *)tstate);

‎Python/_warnings.c‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "pycore_traceback.h" // _Py_DisplaySourceLine()
1010
#include "pycore_tuple.h" // _PyTuple_FromPair
1111
#include "pycore_unicodeobject.h" // _PyUnicode_EqualToASCIIString()
12+
#include "pycore_warnings.h" // _PyWarnings_AfterFork()
1213

1314
#include <stdbool.h>
1415
#include "clinic/_warnings.c.h"
@@ -276,6 +277,23 @@ warnings_lock_held(WarningsState *st)
276277
return PyMutex_IsLocked(&st->lock.mutex);
277278
}
278279

280+
#ifdef HAVE_FORK
281+
void
282+
_PyWarnings_BeforeFork(PyInterpreterState *interp)
283+
{
284+
WarningsState *st = warnings_get_state(interp);
285+
st->lock_held_at_fork = _PyRecursiveMutex_IsLockedByCurrentThread(&st->lock);
286+
}
287+
288+
void
289+
_PyWarnings_AfterFork(PyInterpreterState *interp)
290+
{
291+
WarningsState *st = warnings_get_state(interp);
292+
_PyRecursiveMutex_at_fork_reinit(&st->lock, st->lock_held_at_fork);
293+
st->lock_held_at_fork = false;
294+
}
295+
#endif
296+
279297
static PyObject *
280298
get_warnings_context(PyInterpreterState *interp)
281299
{

‎Python/lock.c‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,25 @@ _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m)
453453
return 0;
454454
}
455455

456+
#ifdef HAVE_FORK
457+
// Reinitialize the mutex in the child process after fork(). The caller must
458+
// record before fork() whether the forking thread owns the mutex, since the
459+
// thread ident can change in the child (gh-126688). If it does, the child
460+
// keeps the mutex with the same recursion level; otherwise the owner and any
461+
// waiters no longer exist in the child and the mutex is reset.
462+
void
463+
_PyRecursiveMutex_at_fork_reinit(_PyRecursiveMutex *m, int owned)
464+
{
465+
if (owned) {
466+
m->mutex = (PyMutex){._bits = _Py_LOCKED};
467+
_Py_atomic_store_ullong_relaxed(&m->thread, PyThread_get_thread_ident_ex());
468+
}
469+
else {
470+
memset(m, 0, sizeof(*m));
471+
}
472+
}
473+
#endif
474+
456475
#define _Py_WRITE_LOCKED 1
457476
#define _PyRWMutex_READER_SHIFT 2
458477
#define _Py_RWMUTEX_MAX_READERS (UINTPTR_MAX >> _PyRWMutex_READER_SHIFT)

0 commit comments

Comments
 (0)