Skip to content

Commit 42ba6e7

Browse files
committed
gh-87135: threading.Lock: Raise rather than hang on Python finalization
After Python finalization gets to the point where no other thread can attach thread state, attempting to acquire a Python lock must hang. Raise PythonFinalizationError instead of hanging.
1 parent ffb2a02 commit 42ba6e7

4 files changed

Lines changed: 72 additions & 2 deletions

File tree

Include/internal/pycore_lock.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ typedef enum _PyLockFlags {
5151

5252
// Fail if interrupted by a signal while waiting on the lock.
5353
_PY_FAIL_IF_INTERRUPTED = 4,
54+
55+
// Locking & unlocking this lock requires attached thread state.
56+
// If locking returns PY_LOCK_FAILURE, a Python exception *may* be raised.
57+
// Implies _PY_LOCK_HANDLE_SIGNALS and _PY_LOCK_DETACH.
58+
_PY_LOCK_PYTHONLOCK = 8 | 2 | 1,
5459
} _PyLockFlags;
5560

5661
// Lock a mutex with an optional timeout and additional options. See

Lib/test/test_threading.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,51 @@ def __del__(self):
12471247
self.assertEqual(err, b"")
12481248
self.assertIn(b"all clear", out)
12491249

1250+
def test_acquire_daemon_thread_lock_in_finalization(self):
1251+
# gh-123940: Py_Finalize() prevents other threads from running Python
1252+
# code (and so, releasing locks), so acquiring a locked lock can not
1253+
# succeed.
1254+
# We raise an exception rather than hang.
1255+
for timeout in (None, 10):
1256+
with self.subTest(timeout=timeout):
1257+
code = textwrap.dedent(f"""
1258+
import threading
1259+
import time
1260+
1261+
thread_started_event = threading.Event()
1262+
1263+
lock = threading.Lock()
1264+
def loop():
1265+
with lock:
1266+
thread_started_event.set()
1267+
while True:
1268+
time.sleep(1)
1269+
1270+
class Cycle:
1271+
def __init__(self):
1272+
self.self_ref = self
1273+
self.thr = threading.Thread(
1274+
target=loop, daemon=True)
1275+
self.thr.start()
1276+
thread_started_event.wait()
1277+
1278+
def __del__(self):
1279+
assert self.thr.is_alive()
1280+
try:
1281+
lock.acquire()
1282+
except PythonFinalizationError:
1283+
assert self.thr.is_alive()
1284+
print('got the correct exception!')
1285+
1286+
# Cycle holds a reference to itself, which ensures it is
1287+
# cleaned up during the GC that runs after daemon threads
1288+
# have been forced to exit during finalization.
1289+
Cycle()
1290+
""")
1291+
rc, out, err = assert_python_ok("-c", code)
1292+
self.assertEqual(err, b"")
1293+
self.assertIn(b"got the correct exception", out)
1294+
12501295
def test_start_new_thread_failed(self):
12511296
# gh-109746: if Python fails to start newly created thread
12521297
# due to failure of underlying PyThread_start_new_thread() call,

Modules/_threadmodule.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -835,8 +835,12 @@ lock_PyThread_acquire_lock(PyObject *op, PyObject *args, PyObject *kwds)
835835
}
836836

837837
PyLockStatus r = _PyMutex_LockTimed(&self->lock, timeout,
838-
_PY_LOCK_HANDLE_SIGNALS | _PY_LOCK_DETACH);
838+
_PY_LOCK_PYTHONLOCK);
839839
if (r == PY_LOCK_INTR) {
840+
assert(PyErr_Occurred());
841+
return NULL;
842+
}
843+
if (r == PY_LOCK_FAILURE && PyErr_Occurred()) {
840844
return NULL;
841845
}
842846

@@ -1055,8 +1059,12 @@ rlock_acquire(PyObject *op, PyObject *args, PyObject *kwds)
10551059
}
10561060

10571061
PyLockStatus r = _PyRecursiveMutex_LockTimed(&self->lock, timeout,
1058-
_PY_LOCK_HANDLE_SIGNALS | _PY_LOCK_DETACH);
1062+
_PY_LOCK_PYTHONLOCK);
10591063
if (r == PY_LOCK_INTR) {
1064+
assert(PyErr_Occurred());
1065+
return NULL;
1066+
}
1067+
if (r == PY_LOCK_FAILURE && PyErr_Occurred()) {
10601068
return NULL;
10611069
}
10621070

Python/lock.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,18 @@ _PyMutex_LockTimed(PyMutex *m, PyTime_t timeout, _PyLockFlags flags)
9595
if (timeout == 0) {
9696
return PY_LOCK_FAILURE;
9797
}
98+
if ((flags & _PY_LOCK_PYTHONLOCK) && Py_IsFinalizing()) {
99+
// At this phase of runtime shutdown, only the finalization thread
100+
// can have attached thread state; others hang if they try
101+
// attaching. And since operations on this lock requires attached
102+
// thread state (_PY_LOCK_PYTHONLOCK), the finalization thread is
103+
// running this code, and no other thread can unlock.
104+
// Raise rather than hang. (_PY_LOCK_PYTHONLOCK allows raising
105+
// exceptons.)
106+
PyErr_SetString(PyExc_PythonFinalizationError,
107+
"cannot acquire lock at interpreter finalization");
108+
return PY_LOCK_FAILURE;
109+
}
98110

99111
uint8_t newv = v;
100112
if (!(v & _Py_HAS_PARKED)) {

0 commit comments

Comments
 (0)