Skip to content

Commit 8013779

Browse files
authored
gh-158973: Reset the warnings lock after fork in the child (gh-158974)
1 parent a2c999b commit 8013779

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
@@ -650,6 +650,10 @@ struct _warnings_runtime_state {
650650
PyObject *once_registry; /* Dict */
651651
PyObject *default_action; /* String */
652652
_PyRecursiveMutex lock;
653+
#ifdef HAVE_FORK
654+
// Whether the thread calling fork() owns the warnings lock.
655+
bool lock_held_at_fork;
656+
#endif
653657
long filters_version;
654658
PyObject *context;
655659
};

‎Include/internal/pycore_lock.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,9 @@ PyAPI_FUNC(void) _PyRecursiveMutex_Lock(_PyRecursiveMutex *m);
173173
extern PyLockStatus _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags flags);
174174
PyAPI_FUNC(void) _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m);
175175
extern int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m);
176+
#ifdef HAVE_FORK
177+
extern void _PyRecursiveMutex_at_fork_reinit(_PyRecursiveMutex *m, int owned);
178+
#endif
176179

177180
// A readers-writer (RW) lock. The lock supports multiple concurrent readers or
178181
// 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
@@ -15,6 +15,7 @@
1515
from test.support import import_helper
1616
from test.support import isolation
1717
from test.support import os_helper
18+
from test.support import threading_helper
1819
from test.support import warnings_helper
1920
from test.support import force_not_colorized
2021
from test.support.script_helper import assert_python_ok, assert_python_failure
@@ -1632,6 +1633,100 @@ def test_release_lock_no_lock(self):
16321633
):
16331634
c_warnings._release_lock()
16341635

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

16361731
class _DeprecatedTest(BaseTest, unittest.TestCase):
16371732

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()
@@ -722,6 +723,7 @@ PyOS_BeforeFork(void)
722723
_PyImport_AcquireLock(interp);
723724
_PyEval_StopTheWorldAll(&_PyRuntime);
724725
HEAD_LOCK(&_PyRuntime);
726+
_PyWarnings_BeforeFork(interp);
725727
}
726728

727729
void
@@ -792,6 +794,9 @@ PyOS_AfterFork_Child(void)
792794
_Py_jit_debug_mutex = (PyMutex){0};
793795
#endif
794796

797+
// Thread-state cleanup can run finalizers that issue warnings.
798+
_PyWarnings_AfterFork(tstate->interp);
799+
795800
reset_remotedebug_data(tstate);
796801

797802
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)