Skip to content

Commit 84990b5

Browse files
naschemengoldbaum
andauthored
[3.14] gh-158973: Reset the warnings lock after fork in the child (gh-158974) (gh-159615)
Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
1 parent 7e03ae8 commit 84990b5

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
@@ -157,6 +157,9 @@ PyAPI_FUNC(void) _PyRecursiveMutex_Lock(_PyRecursiveMutex *m);
157157
extern PyLockStatus _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags flags);
158158
PyAPI_FUNC(void) _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m);
159159
extern int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m);
160+
#ifdef HAVE_FORK
161+
extern void _PyRecursiveMutex_at_fork_reinit(_PyRecursiveMutex *m, int owned);
162+
#endif
160163

161164
// A readers-writer (RW) lock. The lock supports multiple concurrent readers or
162165
// 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
@@ -1561,6 +1562,105 @@ def test_release_lock_no_lock(self):
15611562
):
15621563
c_warnings._release_lock()
15631564

1565+
@support.cpython_only
1566+
@unittest.skipUnless(support.has_fork_support, 'requires working os.fork')
1567+
@threading_helper.requires_working_threading()
1568+
def test_fork_other_thread_holds_lock(self):
1569+
code = textwrap.dedent('''
1570+
import _warnings
1571+
import os
1572+
import threading
1573+
import warnings
1574+
from test import support
1575+
1576+
parent_pid = os.getpid()
1577+
read_fd, write_fd = os.pipe()
1578+
local = threading.local()
1579+
ready = threading.Event()
1580+
release = threading.Event()
1581+
warnings.simplefilter('ignore', DeprecationWarning)
1582+
warnings.simplefilter('ignore', ResourceWarning)
1583+
1584+
class WarnOnDelete:
1585+
def __del__(self):
1586+
if os.getpid() != parent_pid:
1587+
# Even an ignored warning must acquire the lock.
1588+
warnings.warn('child cleanup', ResourceWarning)
1589+
os.write(write_fd, b'finalized')
1590+
1591+
def worker():
1592+
local.obj = WarnOnDelete()
1593+
_warnings._acquire_lock()
1594+
_warnings._acquire_lock()
1595+
try:
1596+
ready.set()
1597+
release.wait()
1598+
finally:
1599+
_warnings._release_lock()
1600+
_warnings._release_lock()
1601+
1602+
thread = threading.Thread(target=worker)
1603+
thread.start()
1604+
# Release the worker before fork's parent-side warning attempts
1605+
# to acquire the warnings lock.
1606+
os.register_at_fork(after_in_parent=release.set)
1607+
try:
1608+
assert ready.wait(support.LONG_TIMEOUT)
1609+
pid = os.fork()
1610+
if pid == 0:
1611+
# The finalizer must run before os.fork() returns.
1612+
os._exit(0)
1613+
support.wait_process(pid, exitcode=0)
1614+
finally:
1615+
release.set()
1616+
thread.join()
1617+
os.close(write_fd)
1618+
assert os.read(read_fd, 100) == b'finalized'
1619+
os.close(read_fd)
1620+
''')
1621+
assert_python_ok('-c', code)
1622+
1623+
@support.cpython_only
1624+
@unittest.skipUnless(support.has_fork_support, 'requires working os.fork')
1625+
def test_fork_current_thread_holds_lock(self):
1626+
code = textwrap.dedent('''
1627+
import _warnings
1628+
import os
1629+
import warnings
1630+
from test import support
1631+
1632+
warnings.simplefilter('ignore')
1633+
for _ in range(3):
1634+
_warnings._acquire_lock()
1635+
pid = os.fork()
1636+
if pid == 0:
1637+
# Ownership must also survive a second fork in the child.
1638+
child_pid = os.fork()
1639+
if child_pid != 0:
1640+
support.wait_process(child_pid, exitcode=0)
1641+
try:
1642+
# A warning must not change the inherited recursion depth.
1643+
warnings.warn('after fork')
1644+
for _ in range(3):
1645+
_warnings._release_lock()
1646+
try:
1647+
_warnings._release_lock()
1648+
except RuntimeError:
1649+
pass
1650+
else:
1651+
raise AssertionError('unexpected recursion depth')
1652+
_warnings._acquire_lock()
1653+
_warnings._release_lock()
1654+
except BaseException:
1655+
if pid == 0:
1656+
os._exit(1)
1657+
raise
1658+
if pid == 0:
1659+
os._exit(0)
1660+
support.wait_process(pid, exitcode=0)
1661+
''')
1662+
assert_python_ok('-c', code)
1663+
15641664

15651665
class _DeprecatedTest(BaseTest, unittest.TestCase):
15661666

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
@@ -29,6 +29,7 @@
2929
#include "pycore_signal.h" // Py_NSIG
3030
#include "pycore_time.h" // _PyLong_FromTime_t()
3131
#include "pycore_typeobject.h" // _PyType_AddMethod()
32+
#include "pycore_warnings.h" // _PyWarnings_AfterFork()
3233

3334
#ifndef MS_WINDOWS
3435
# include "posixmodule.h" // _PyLong_FromUid()
@@ -669,6 +670,7 @@ PyOS_BeforeFork(void)
669670
_PyImport_AcquireLock(interp);
670671
_PyEval_StopTheWorldAll(&_PyRuntime);
671672
HEAD_LOCK(&_PyRuntime);
673+
_PyWarnings_BeforeFork(interp);
672674
}
673675

674676
void
@@ -732,6 +734,9 @@ PyOS_AfterFork_Child(void)
732734
goto fatal_error;
733735
}
734736

737+
// Thread-state cleanup can run finalizers that issue warnings.
738+
_PyWarnings_AfterFork(tstate->interp);
739+
735740
reset_remotedebug_data(tstate);
736741

737742
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_sysmodule.h" // _PySys_GetOptionalAttr()
1010
#include "pycore_traceback.h" // _Py_DisplaySourceLine()
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"
@@ -264,6 +265,28 @@ warnings_lock_held(WarningsState *st)
264265
return PyMutex_IsLocked(&st->lock.mutex);
265266
}
266267

268+
#ifdef HAVE_FORK
269+
// PyOS_BeforeFork() holds HEAD_LOCK while saving this value and until fork(),
270+
// so concurrent forks cannot overwrite it before the child inherits it.
271+
// The child consumes its private copy before running Python callbacks.
272+
static bool warnings_lock_held_at_fork;
273+
274+
void
275+
_PyWarnings_BeforeFork(PyInterpreterState *interp)
276+
{
277+
WarningsState *st = warnings_get_state(interp);
278+
warnings_lock_held_at_fork = _PyRecursiveMutex_IsLockedByCurrentThread(&st->lock);
279+
}
280+
281+
void
282+
_PyWarnings_AfterFork(PyInterpreterState *interp)
283+
{
284+
WarningsState *st = warnings_get_state(interp);
285+
_PyRecursiveMutex_at_fork_reinit(&st->lock, warnings_lock_held_at_fork);
286+
warnings_lock_held_at_fork = false;
287+
}
288+
#endif
289+
267290
static PyObject *
268291
get_warnings_context(PyInterpreterState *interp)
269292
{

‎Python/lock.c‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,25 @@ _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m)
432432
return 0;
433433
}
434434

435+
#ifdef HAVE_FORK
436+
// Reinitialize the mutex in the child process after fork(). The caller must
437+
// record before fork() whether the forking thread owns the mutex, since the
438+
// thread ident can change in the child (gh-126688). If it does, the child
439+
// keeps the mutex with the same recursion level; otherwise the owner and any
440+
// waiters no longer exist in the child and the mutex is reset.
441+
void
442+
_PyRecursiveMutex_at_fork_reinit(_PyRecursiveMutex *m, int owned)
443+
{
444+
if (owned) {
445+
m->mutex = (PyMutex){._bits = _Py_LOCKED};
446+
_Py_atomic_store_ullong_relaxed(&m->thread, PyThread_get_thread_ident_ex());
447+
}
448+
else {
449+
memset(m, 0, sizeof(*m));
450+
}
451+
}
452+
#endif
453+
435454
#define _Py_WRITE_LOCKED 1
436455
#define _PyRWMutex_READER_SHIFT 2
437456
#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
@@ -353,6 +353,7 @@ Objects/unicodeobject.c unicode_translate_call_errorhandler argparse -
353353
Parser/parser.c - reserved_keywords -
354354
Parser/parser.c - soft_keywords -
355355
Parser/lexer/lexer.c - type_comment_prefix -
356+
Python/_warnings.c - warnings_lock_held_at_fork -
356357
Python/ceval.c - _PyEval_BinaryOps -
357358
Python/ceval.c - _Py_INTERPRETER_TRAMPOLINE_INSTRUCTIONS -
358359
Python/codecs.c - Py_hexdigits -

0 commit comments

Comments
 (0)