Skip to content

Commit 63ad59b

Browse files
committed
gh-158973: Reset the warnings lock after fork in the child
1 parent 1b015e6 commit 63ad59b

6 files changed

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

‎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()
@@ -716,6 +717,7 @@ PyOS_BeforeFork(void)
716717
_PyImport_AcquireLock(interp);
717718
_PyEval_StopTheWorldAll(&_PyRuntime);
718719
HEAD_LOCK(&_PyRuntime);
720+
_PyWarnings_BeforeFork(interp);
719721
}
720722

721723
void
@@ -786,6 +788,9 @@ PyOS_AfterFork_Child(void)
786788
_Py_jit_debug_mutex = (PyMutex){0};
787789
#endif
788790

791+
// Thread-state cleanup can run finalizers that issue warnings.
792+
_PyWarnings_AfterFork(tstate->interp);
793+
789794
reset_remotedebug_data(tstate);
790795

791796
reset_asyncio_state((_PyThreadStateImpl *)tstate);

‎Python/_warnings.c‎

Lines changed: 28 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,33 @@ 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+
// Record ownership before fork(): the thread ID can change in the child.
286+
st->lock_held_at_fork = _PyRecursiveMutex_IsLockedByCurrentThread(&st->lock);
287+
}
288+
289+
void
290+
_PyWarnings_AfterFork(PyInterpreterState *interp)
291+
{
292+
WarningsState *st = warnings_get_state(interp);
293+
if (st->lock_held_at_fork) {
294+
// The surviving thread will still release the lock as its stack
295+
// unwinds. Preserve the recursion depth, but discard dead waiters.
296+
st->lock.mutex = (PyMutex){._bits = _Py_LOCKED};
297+
st->lock.thread = PyThread_get_thread_ident_ex();
298+
}
299+
else {
300+
// The owner (if any) no longer exists in the child.
301+
st->lock = (_PyRecursiveMutex){0};
302+
}
303+
st->lock_held_at_fork = false;
304+
}
305+
#endif
306+
279307
static PyObject *
280308
get_warnings_context(PyInterpreterState *interp)
281309
{

0 commit comments

Comments
 (0)