Skip to content

Commit 37baa8f

Browse files
committed
gh-159485: Fix warnings lock deadlock during interpreter shutdown
1 parent 938144e commit 37baa8f

8 files changed

Lines changed: 109 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 @@ typedef struct {
168168
size_t level;
169169
} _PyRecursiveMutex;
170170

171+
// All other threads that use the mutex must be barred from reattaching.
172+
extern void _PyRecursiveMutex_at_shutdown_reinit(_PyRecursiveMutex *m);
173+
171174
PyAPI_FUNC(int) _PyRecursiveMutex_IsLockedByCurrentThread(_PyRecursiveMutex *m);
172175
PyAPI_FUNC(void) _PyRecursiveMutex_Lock(_PyRecursiveMutex *m);
173176
extern PyLockStatus _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags flags);

‎Include/internal/pycore_warnings.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ extern "C" {
88
# error "this header requires Py_BUILD_CORE define"
99
#endif
1010

11+
extern void _PyWarnings_PrepareForFinalization(PyInterpreterState *interp);
12+
1113
extern int _PyWarnings_InitState(PyInterpreterState *interp);
1214

1315
extern PyObject* _PyWarnings_Init(void);

‎Lib/test/test_embed.py‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,68 @@ def test_datetime_reset_strptime(self):
507507
out, err = self.run_embedded_interpreter("test_repeated_init_exec", code)
508508
self.assertEqual(out, '20000101\n' * INIT_LOOPS)
509509

510+
@threading_helper.requires_working_threading()
511+
@unittest.skipUnless(_testinternalcapi, "requires _testinternalcapi")
512+
def test_warnings_at_shutdown(self):
513+
import_helper.import_module('_testcapi')
514+
code = textwrap.dedent('''
515+
import _testinternalcapi
516+
import _warnings
517+
import threading
518+
import warnings
519+
from test import support
520+
521+
warnings.simplefilter('ignore')
522+
local = threading.local()
523+
ready = threading.Event()
524+
525+
class WarnOnDelete:
526+
def __del__(self):
527+
warnings.warn('during shutdown', ResourceWarning)
528+
if not ready.is_set():
529+
# The finalizing thread still owns all three levels.
530+
for _ in range(3):
531+
_warnings._release_lock()
532+
try:
533+
_warnings._release_lock()
534+
except RuntimeError:
535+
pass
536+
else:
537+
raise AssertionError('unexpected recursion depth')
538+
print('finalized')
539+
540+
def worker():
541+
local.obj = WarnOnDelete()
542+
_warnings._acquire_lock()
543+
ready.set()
544+
threading.Event().wait()
545+
546+
for _ in range(3):
547+
_warnings._acquire_lock()
548+
threading.Thread(target=worker, daemon=True).start()
549+
for _ in support.sleeping_retry(4):
550+
if _testinternalcapi.warnings_lock_has_waiters():
551+
break
552+
''')
553+
release = textwrap.dedent('''
554+
for _ in range(3):
555+
_warnings._release_lock()
556+
assert ready.wait(4), 'lock not acquired'
557+
''')
558+
subinterp = (
559+
'from test import support\n'
560+
f'assert support.run_in_subinterp({code + release!r}) == 0'
561+
)
562+
# Cover Py_EndInterpreter(), then shut down with a waiter. Reuse the
563+
# main interpreter's mutex and shut down with a daemon owning it.
564+
# All three thread-local finalizers must warn.
565+
out, err = self.run_embedded_interpreter(
566+
"test_repeated_init_exec", subinterp + '\n' + code, code + release,
567+
env=remove_python_envvars(),
568+
timeout=10)
569+
self.assertEqual(out, 'finalized\n' * 3)
570+
self.assertEqual(err, '--- Loop #1 ---\n--- Loop #2 ---\n')
571+
510572
def test_static_types_inherited_slots(self):
511573
script = textwrap.dedent("""
512574
import test.support
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Prevent a deadlock when emitting warnings during late interpreter shutdown
2+
if a daemon thread holds the warnings lock or is waiting to acquire it.

‎Modules/_testinternalcapi/test_lock.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
// C Extension module to test pycore_lock.h API
22

33
#include "parts.h"
4+
#include "pycore_interp.h" // PyInterpreterState.warnings
45
#include "pycore_lock.h"
6+
#include "pycore_pystate.h" // _PyEval_StopTheWorld()
57
#include "pycore_pythread.h" // PyThread_get_thread_ident_ex()
68

79
#include "clinic/test_lock.c.h"
@@ -549,7 +551,21 @@ test_lock_recursive(PyObject *self, PyObject *obj)
549551
Py_RETURN_NONE;
550552
}
551553

554+
static PyObject *
555+
warnings_lock_has_waiters(PyObject *self, PyObject *Py_UNUSED(ignored))
556+
{
557+
PyInterpreterState *interp = PyInterpreterState_Get();
558+
PyMutex *mutex = &interp->warnings.lock.mutex;
559+
// A contender sets the parked bit before enqueuing, but cannot detach
560+
// until it has enqueued. Stop the world to close that window on nogil.
561+
_PyEval_StopTheWorld(interp);
562+
uint8_t bits = _Py_atomic_load_uint8_relaxed(&mutex->_bits);
563+
_PyEval_StartTheWorld(interp);
564+
return PyBool_FromLong((bits & _Py_HAS_PARKED) != 0);
565+
}
566+
552567
static PyMethodDef test_methods[] = {
568+
{"warnings_lock_has_waiters", warnings_lock_has_waiters, METH_NOARGS},
553569
{"test_lock_basic", test_lock_basic, METH_NOARGS},
554570
{"test_lock_two_threads", test_lock_two_threads, METH_NOARGS},
555571
{"test_lock_counter", test_lock_counter, METH_NOARGS},

‎Python/_warnings.c‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,13 @@ get_warnings_attr(PyInterpreterState *interp, PyObject *attr, int try_import)
254254
return obj;
255255
}
256256

257+
void
258+
_PyWarnings_PrepareForFinalization(PyInterpreterState *interp)
259+
{
260+
WarningsState *st = warnings_get_state(interp);
261+
_PyRecursiveMutex_at_shutdown_reinit(&st->lock);
262+
}
263+
257264
static inline void
258265
warnings_lock(PyInterpreterState *interp)
259266
{

‎Python/lock.c‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,21 @@ _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags
427427
return s;
428428
}
429429

430+
void
431+
_PyRecursiveMutex_at_shutdown_reinit(_PyRecursiveMutex *m)
432+
{
433+
// Preserve the finalizing thread's ownership and recursion depth.
434+
if (_PyRecursiveMutex_IsLockedByCurrentThread(m)) {
435+
m->mutex = (PyMutex){._bits = _Py_LOCKED};
436+
}
437+
else {
438+
memset(m, 0, sizeof(*m));
439+
}
440+
// Remove parked waiters from the queue. After waking, they will hang
441+
// when they try to reattach their thread state.
442+
_PyParkingLot_UnparkAll(&m->mutex._bits);
443+
}
444+
430445
void
431446
_PyRecursiveMutex_Unlock(_PyRecursiveMutex *m)
432447
{

‎Python/pylifecycle.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2452,6 +2452,7 @@ _Py_Finalize(_PyRuntimeState *runtime)
24522452
for (PyThreadState *p = list; p != NULL; p = p->next) {
24532453
_PyThreadState_SetShuttingDown(p);
24542454
}
2455+
_PyWarnings_PrepareForFinalization(tstate->interp);
24552456
_PyEval_StartTheWorldAll(runtime);
24562457
PyMutex_Unlock(&tstate->interp->ceval.pending.mutex);
24572458

@@ -2834,6 +2835,7 @@ Py_EndInterpreter(PyThreadState *tstate)
28342835
_PyThreadState_SetShuttingDown(p);
28352836
}
28362837

2838+
_PyWarnings_PrepareForFinalization(interp);
28372839
_PyEval_StartTheWorldAll(interp->runtime);
28382840
PyMutex_Unlock(&interp->ceval.pending.mutex);
28392841
_PyThreadState_DeleteList(list, /*is_after_fork=*/0);

0 commit comments

Comments
 (0)