Skip to content

Commit 81fe70e

Browse files
naschemengoldbaum
andauthored
[3.15] gh-158973: Reset the warnings lock after fork in the child (gh-158974) (gh-159614)
Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
1 parent 40abc65 commit 81fe70e

8 files changed

Lines changed: 159 additions & 0 deletions

File tree

‎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: 100 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,105 @@ 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+
if pid == 0:
1697+
# Ownership must also survive a second fork in the child.
1698+
child_pid = os.fork()
1699+
if child_pid != 0:
1700+
support.wait_process(child_pid, exitcode=0)
1701+
try:
1702+
# A warning must not change the inherited recursion depth.
1703+
warnings.warn('after fork')
1704+
for _ in range(3):
1705+
_warnings._release_lock()
1706+
try:
1707+
_warnings._release_lock()
1708+
except RuntimeError:
1709+
pass
1710+
else:
1711+
raise AssertionError('unexpected recursion depth')
1712+
_warnings._acquire_lock()
1713+
_warnings._release_lock()
1714+
except BaseException:
1715+
if pid == 0:
1716+
os._exit(1)
1717+
raise
1718+
if pid == 0:
1719+
os._exit(0)
1720+
support.wait_process(pid, exitcode=0)
1721+
''')
1722+
assert_python_ok('-c', code)
1723+
16241724

16251725
class _DeprecatedTest(BaseTest, unittest.TestCase):
16261726

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: 23 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,28 @@ warnings_lock_held(WarningsState *st)
276277
return PyMutex_IsLocked(&st->lock.mutex);
277278
}
278279

280+
#ifdef HAVE_FORK
281+
// PyOS_BeforeFork() holds HEAD_LOCK while saving this value and until fork(),
282+
// so concurrent forks cannot overwrite it before the child inherits it.
283+
// The child consumes its private copy before running Python callbacks.
284+
static bool warnings_lock_held_at_fork;
285+
286+
void
287+
_PyWarnings_BeforeFork(PyInterpreterState *interp)
288+
{
289+
WarningsState *st = warnings_get_state(interp);
290+
warnings_lock_held_at_fork = _PyRecursiveMutex_IsLockedByCurrentThread(&st->lock);
291+
}
292+
293+
void
294+
_PyWarnings_AfterFork(PyInterpreterState *interp)
295+
{
296+
WarningsState *st = warnings_get_state(interp);
297+
_PyRecursiveMutex_at_fork_reinit(&st->lock, warnings_lock_held_at_fork);
298+
warnings_lock_held_at_fork = false;
299+
}
300+
#endif
301+
279302
static PyObject *
280303
get_warnings_context(PyInterpreterState *interp)
281304
{

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

‎Tools/c-analyzer/cpython/ignored.tsv‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,7 @@ Objects/unicodeobject.c unicode_translate_call_errorhandler argparse -
362362
Parser/parser.c - reserved_keywords -
363363
Parser/parser.c - soft_keywords -
364364
Parser/lexer/lexer.c - type_comment_prefix -
365+
Python/_warnings.c - warnings_lock_held_at_fork -
365366
Python/ceval.c - _PyEval_BinaryOps -
366367
Python/ceval.c - _Py_INTERPRETER_TRAMPOLINE_INSTRUCTIONS -
367368
Python/ceval.c - _Py_INTERPRETER_TRAMPOLINE_INSTRUCTIONS_PTR -

0 commit comments

Comments
 (0)