Skip to content

Commit 50fcb91

Browse files
naschemenascheme
authored
gh-155400: Fix deadlock with type_lock_prevent_release() (#155401)
1 parent fbd2e01 commit 50fcb91

3 files changed

Lines changed: 132 additions & 43 deletions

File tree

‎Lib/test/test_free_threading/test_type.py‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,5 +324,63 @@ def wrapper():
324324
for reader in readers:
325325
reader.join()
326326

327+
def test_concurrent_setattr_deadlock(self):
328+
# gh-155400: two threads assigning to a special method of the same
329+
# class could deadlock. One thread held the type lock and waited for
330+
# the type dict mutex, which its critical section had released when it
331+
# blocked on the stop-the-world mutex, while the other held the type
332+
# dict mutex and waited for the type lock.
333+
# This is fairly difficult to trigger the race but this N seems to do
334+
# it at least sometimes.
335+
N = 200
336+
done = False
337+
338+
class Base:
339+
pass
340+
341+
def setter():
342+
func = lambda self: "x"
343+
barrier.wait()
344+
while not done:
345+
Base.__repr__ = func
346+
try:
347+
del Base.__repr__
348+
except AttributeError:
349+
pass
350+
351+
def subclasser():
352+
barrier.wait()
353+
while not done:
354+
type('Sub', (Base,), {})()
355+
356+
def lister():
357+
barrier.wait()
358+
while not done:
359+
Base.__subclasses__()
360+
361+
def basesetter():
362+
nonlocal done
363+
barrier.wait()
364+
for _ in range(N):
365+
class A:
366+
pass
367+
class C:
368+
pass
369+
class B(A):
370+
pass
371+
B.__bases__ = (C,)
372+
done = True
373+
374+
# The setter threads are the ones that deadlock. The others are there
375+
# to keep the type lock and the stop-the-world mutex contended, which
376+
# is what gets the setters into the window where it happens.
377+
targets = (setter, setter, subclasser, subclasser,
378+
lister, lister, basesetter)
379+
barrier = threading.Barrier(len(targets))
380+
threads = [Thread(target=target) for target in targets]
381+
with threading_helper.start_threads(threads):
382+
pass
383+
384+
327385
if __name__ == "__main__":
328386
unittest.main()

‎Misc/NEWS.d/next/Core_and_Builtins/2026-08-07-18-32-15.gh-issue-155400.Kq3Vpt.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix a deadlock in the free-threaded build between two threads assigning to a
2+
special method of the same class. While applying the type slot updates with
3+
the world stopped, only the type lock was prevented from being released; the
4+
type dict mutex could still be released and re-acquired, in the wrong order,
5+
if the thread blocked on the stop-the-world mutex.

‎Objects/typeobject.c‎

Lines changed: 69 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,13 @@ class object "PyObject *" "&PyBaseObject_Type"
4646
#define NEXT_VERSION_TAG(interp) \
4747
(interp)->types.next_version_tag
4848

49+
// Storage for the mutexes saved by type_lock_prevent_release(). Defined for
50+
// both builds so the call sites don't need to be conditionally compiled.
51+
typedef struct {
52+
PyMutex *mutex1;
53+
PyMutex *mutex2;
54+
} pinned_mutexes_t;
55+
4956
#ifdef Py_GIL_DISABLED
5057

5158
// There's a global lock for types that ensures that tp_version_tag and
@@ -124,44 +131,54 @@ types_start_world(void)
124131
assert(!types_world_is_stopped());
125132
}
126133

127-
// This is used to temporarily prevent the TYPE_LOCK from being suspended
128-
// when held by the topmost critical section.
134+
// Temporarily prevent the mutexes held by the topmost critical section from
135+
// being released when the current thread blocks (blocking detaches the thread,
136+
// which suspends its critical sections and releases the mutexes they hold).
137+
//
138+
// All of the mutexes held by the critical section are pinned, not just
139+
// TYPE_LOCK. If only TYPE_LOCK was pinned then _PyCriticalSection_Resume()
140+
// would have to re-acquire the other mutex while TYPE_LOCK is held. That
141+
// deadlocks against a thread that holds that mutex and is waiting for
142+
// TYPE_LOCK, which is exactly what BEGIN_TYPE_DICT_LOCK() does: the type dict
143+
// mutex is on the heap and TYPE_LOCK is in _PyRuntime, so the address ordering
144+
// used by two-mutex critical sections usually acquires the dict mutex first.
145+
// By pinning both mutexes there is nothing to re-acquire on resume.
146+
//
147+
// Holding the mutexes while blocked does not prevent the world from being
148+
// stopped: a thread waiting on either of them parks with _PY_LOCK_DETACH and
149+
// so is detached while it waits.
129150
static void
130-
type_lock_prevent_release(void)
151+
type_lock_prevent_release(pinned_mutexes_t *pinned)
131152
{
132153
PyThreadState *tstate = _PyThreadState_GET();
133-
uintptr_t *tagptr = &tstate->critical_section;
134-
PyCriticalSection *c = (PyCriticalSection *)(*tagptr & ~_Py_CRITICAL_SECTION_MASK);
135-
if (!(*tagptr & _Py_CRITICAL_SECTION_TWO_MUTEXES)) {
136-
assert(c->_cs_mutex == TYPE_LOCK);
137-
c->_cs_mutex = NULL;
138-
}
139-
else {
154+
uintptr_t tag = tstate->critical_section;
155+
PyCriticalSection *c = (PyCriticalSection *)(tag & ~_Py_CRITICAL_SECTION_MASK);
156+
pinned->mutex1 = c->_cs_mutex;
157+
pinned->mutex2 = NULL;
158+
c->_cs_mutex = NULL;
159+
if ((tag & _Py_CRITICAL_SECTION_TWO_MUTEXES) != 0) {
140160
PyCriticalSection2 *c2 = (PyCriticalSection2 *)c;
141-
if (c->_cs_mutex == TYPE_LOCK) {
142-
c->_cs_mutex = c2->_cs_mutex2;
143-
c2->_cs_mutex2 = NULL;
144-
} else {
145-
assert(c2->_cs_mutex2 == TYPE_LOCK);
146-
c2->_cs_mutex2 = NULL;
147-
}
161+
pinned->mutex2 = c2->_cs_mutex2;
162+
c2->_cs_mutex2 = NULL;
148163
}
164+
assert(pinned->mutex1 == TYPE_LOCK || pinned->mutex2 == TYPE_LOCK);
149165
}
150166

151167
static void
152-
type_lock_allow_release(void)
168+
type_lock_allow_release(pinned_mutexes_t *pinned)
153169
{
154170
PyThreadState *tstate = _PyThreadState_GET();
155-
uintptr_t *tagptr = &tstate->critical_section;
156-
PyCriticalSection *c = (PyCriticalSection *)(*tagptr & ~_Py_CRITICAL_SECTION_MASK);
157-
if (!(*tagptr & _Py_CRITICAL_SECTION_TWO_MUTEXES)) {
158-
assert(c->_cs_mutex == NULL);
159-
c->_cs_mutex = TYPE_LOCK;
160-
}
161-
else {
171+
uintptr_t tag = tstate->critical_section;
172+
PyCriticalSection *c = (PyCriticalSection *)(tag & ~_Py_CRITICAL_SECTION_MASK);
173+
assert(c->_cs_mutex == NULL);
174+
c->_cs_mutex = pinned->mutex1;
175+
if ((tag & _Py_CRITICAL_SECTION_TWO_MUTEXES) != 0) {
162176
PyCriticalSection2 *c2 = (PyCriticalSection2 *)c;
163177
assert(c2->_cs_mutex2 == NULL);
164-
c2->_cs_mutex2 = TYPE_LOCK;
178+
c2->_cs_mutex2 = pinned->mutex2;
179+
}
180+
else {
181+
assert(pinned->mutex2 == NULL);
165182
}
166183
}
167184

@@ -178,8 +195,8 @@ type_lock_allow_release(void)
178195
#define types_world_is_stopped() 1
179196
#define types_stop_world()
180197
#define types_start_world()
181-
#define type_lock_prevent_release()
182-
#define type_lock_allow_release()
198+
#define type_lock_prevent_release(pinned) ((void)(pinned))
199+
#define type_lock_allow_release(pinned) ((void)(pinned))
183200

184201
#endif
185202

@@ -650,14 +667,15 @@ set_tp_mro(PyTypeObject *self, PyObject *mro, int initial)
650667
PyUnstable_Object_EnableDeferredRefcount(mro);
651668
}
652669
}
670+
pinned_mutexes_t pinned;
653671
if (!initial) {
654-
type_lock_prevent_release();
672+
type_lock_prevent_release(&pinned);
655673
types_stop_world();
656674
}
657675
self->tp_mro = mro;
658676
if (!initial) {
659677
types_start_world();
660-
type_lock_allow_release();
678+
type_lock_allow_release(&pinned);
661679
}
662680
}
663681

@@ -1882,13 +1900,14 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, PyTypeObject *b
18821900
PyObject *old_bases = lookup_tp_bases(type);
18831901
assert(old_bases != NULL);
18841902
PyTypeObject *old_base = type->tp_base;
1903+
pinned_mutexes_t pinned;
18851904

1886-
type_lock_prevent_release();
1905+
type_lock_prevent_release(&pinned);
18871906
types_stop_world();
18881907
set_tp_bases(type, Py_NewRef(new_bases), 0);
18891908
type->tp_base = (PyTypeObject *)Py_NewRef(best_base);
18901909
types_start_world();
1891-
type_lock_allow_release();
1910+
type_lock_allow_release(&pinned);
18921911

18931912
PyObject *temp = PyList_New(0);
18941913
if (temp == NULL) {
@@ -1949,12 +1968,12 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, PyTypeObject *b
19491968
if (lookup_tp_bases(type) == new_bases) {
19501969
assert(type->tp_base == best_base);
19511970

1952-
type_lock_prevent_release();
1971+
type_lock_prevent_release(&pinned);
19531972
types_stop_world();
19541973
set_tp_bases(type, old_bases, 0);
19551974
type->tp_base = old_base;
19561975
types_start_world();
1957-
type_lock_allow_release();
1976+
type_lock_allow_release(&pinned);
19581977

19591978
Py_DECREF(new_bases);
19601979
Py_DECREF(best_base);
@@ -3862,16 +3881,22 @@ apply_type_slot_updates(slot_update_t *updates)
38623881
// to update the dict. That's because TYPE_LOCK was acquired using a
38633882
// critical section.
38643883
//
3865-
// The type_lock_prevent_release() call prevents the TYPE_LOCK mutex from
3866-
// being released even if we block on the STM mutex. We need to take care
3867-
// that we do not deadlock because of that. It is safe because we always
3868-
// acquire locks in the same order: first the TYPE_LOCK mutex and then the
3869-
// STM mutex.
3870-
type_lock_prevent_release();
3884+
// The type_lock_prevent_release() call prevents the mutexes held by the
3885+
// critical section (TYPE_LOCK and the type dict mutex) from being released
3886+
// even if we block on the STW mutex. We need to take care that we do not
3887+
// deadlock because of that. It is safe because a thread waiting for either
3888+
// of those mutexes detaches while it waits and so does not hold up the
3889+
// stop-the-world. Pinning both mutexes rather than only TYPE_LOCK is what
3890+
// makes this safe: otherwise the dict mutex would be released when we
3891+
// block and _PyCriticalSection_Resume() would have to re-acquire it while
3892+
// holding TYPE_LOCK, deadlocking with a thread that holds the dict mutex
3893+
// and is waiting for TYPE_LOCK.
3894+
pinned_mutexes_t pinned;
3895+
type_lock_prevent_release(&pinned);
38713896
types_stop_world();
38723897
apply_slot_updates(updates);
38733898
types_start_world();
3874-
type_lock_allow_release();
3899+
type_lock_allow_release(&pinned);
38753900
}
38763901

38773902
#else
@@ -6364,11 +6389,12 @@ _PyType_SetFlagsRecursive(PyTypeObject *self, unsigned long mask, unsigned long
63646389
}
63656390
/* Keep TYPE_LOCK held while waiting for stop-the-world so no thread
63666391
can reassign a version tag before the flag update. */
6367-
type_lock_prevent_release();
6392+
pinned_mutexes_t pinned;
6393+
type_lock_prevent_release(&pinned);
63686394
types_stop_world();
63696395
set_flags_recursive(self, mask, flags);
63706396
types_start_world();
6371-
type_lock_allow_release();
6397+
type_lock_allow_release(&pinned);
63726398
END_TYPE_LOCK();
63736399
}
63746400

0 commit comments

Comments
 (0)