Skip to content

Commit b89f1c9

Browse files
authored
gh-158195: Do not hold the type lock during stop-the-world (gh-159972)
Fix a free-threaded deadlock caused by holding TYPE_LOCK while waiting for the stop-the-world mutex: its owner may need TYPE_LOCK to reattach after running Python code during a type update. Allow TYPE_LOCK to be released while waiting, and keep slot updates and bases validation and publication within a single stop-the-world pause so they cannot become stale. Ensure this work cannot invoke user code or deallocate objects, avoiding re-entrancy while the world is stopped.
1 parent 2ef3728 commit b89f1c9

8 files changed

Lines changed: 857 additions & 442 deletions

File tree

‎Include/internal/pycore_dict.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ PyAPI_FUNC(int) _PyDict_SetItem_KnownHash_LockHeld(PyDictObject *mp, PyObject *k
170170
PyAPI_FUNC(int) _PyDict_GetItemRef_KnownHash_LockHeld(PyDictObject *op, PyObject *key, Py_hash_t hash, PyObject **result);
171171
PyAPI_FUNC(int) _PyDict_GetItemRef_KnownHash(PyDictObject *op, PyObject *key, Py_hash_t hash, PyObject **result);
172172
extern int _PyDict_GetItemRef_Unicode_LockHeld(PyDictObject *op, PyObject *key, PyObject **result);
173+
extern PyObject *_PyDict_GetItem_ExactUnicode(PyDictObject *mp, PyObject *key);
174+
extern int _PyDict_GetItemStackRef_ExactUnicode(PyDictObject *mp,
175+
PyObject *key,
176+
_PyStackRef *value_addr);
173177
PyAPI_FUNC(int) _PyObjectDict_SetItem(PyTypeObject *tp, PyObject *obj, PyObject **dictptr, PyObject *name, PyObject *value);
174178

175179
extern int _PyDict_Pop_KnownHash(

‎Lib/test/test_descr.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5098,6 +5098,31 @@ def __new__(mcls, name, bases, ns):
50985098
with self.assertWarnsRegex(RuntimeWarning, 'MyClass'):
50995099
MyClass = meta('MyClass', (), {})
51005100

5101+
def test_str_subclass_namespace_keys(self):
5102+
# Keys of the class namespace that are str subclasses are copied
5103+
# to exact str in the type's __dict__, like type.__setattr__ does.
5104+
class Key(str):
5105+
def __hash__(self):
5106+
# Collide with the slot name assigned below.
5107+
return hash('__hash__')
5108+
def __eq__(self, other):
5109+
# Must not be called by type dict lookups.
5110+
raise AssertionError('Key.__eq__ called')
5111+
5112+
ns = {Key('other'): 1, '__repr__': lambda self: 'r', 'c': 2}
5113+
C = type('C', (), ns)
5114+
for key in C.__dict__:
5115+
self.assertIs(type(key), str)
5116+
self.assertEqual([str(k) for k in ns], list(C.__dict__)[:3])
5117+
self.assertEqual(repr(C()), 'r')
5118+
self.assertEqual(C.other, 1)
5119+
# Lookups and slot updates that hash-collide with 'other' do not
5120+
# call Key.__eq__.
5121+
C.__hash__ = None
5122+
with self.assertRaises(TypeError):
5123+
hash(C())
5124+
del C.__hash__
5125+
51015126
def test_proxy_call(self):
51025127
class FakeStr:
51035128
__class__ = str

‎Lib/test/test_free_threading/test_threading.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,30 @@ def mutate_thread():
2626

2727

2828
class TestThreadState(unittest.TestCase):
29+
def test_stop_the_world(self):
30+
import _testinternalcapi
31+
32+
_testinternalcapi.stop_the_world()
33+
_testinternalcapi.stop_the_world(0)
34+
35+
def pause():
36+
for _ in range(10):
37+
_testinternalcapi.stop_the_world(0.0001)
38+
39+
threading_helper.run_concurrently(pause, nthreads=2)
40+
for duration in (-1, -0.1, float("nan")):
41+
with self.subTest(duration=duration):
42+
with self.assertRaises(ValueError):
43+
_testinternalcapi.stop_the_world(duration)
44+
for duration in (float("inf"), 10**100):
45+
with self.subTest(duration=duration):
46+
with self.assertRaises(OverflowError):
47+
_testinternalcapi.stop_the_world(duration)
48+
for duration in (None, "0"):
49+
with self.subTest(duration=duration):
50+
with self.assertRaises(TypeError):
51+
_testinternalcapi.stop_the_world(duration)
52+
2953
@support.requires_subprocess()
3054
def test_tight_stw_loop_does_not_starve_attach(self):
3155
script = textwrap.dedent(f"""

‎Lib/test/test_free_threading/test_type.py‎

Lines changed: 171 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import gc
12
import threading
23
import unittest
34

@@ -19,6 +20,24 @@
1920
class A:
2021
attr = 1
2122

23+
def world_stoppers():
24+
"""Finite workloads that contend for the stop-the-world mutex.
25+
26+
Waiting for the mutex releases the type lock, so that other threads
27+
can change types in the meantime. Use a short pause rather than full collections
28+
to prolong the contention without depending on the size of the heap.
29+
"""
30+
def stopper():
31+
for _ in range(100):
32+
_testinternalcapi.stop_the_world()
33+
34+
def pauser():
35+
for _ in range(100):
36+
_testinternalcapi.stop_the_world(0.0001)
37+
38+
return [stopper, pauser]
39+
40+
2241
@threading_helper.requires_working_threading()
2342
class TestType(TestCase):
2443
def test_attr_cache(self):
@@ -325,9 +344,8 @@ def wrapper():
325344
reader.join()
326345

327346
def test_setattr_many_subclasses(self):
328-
# gh-155978: Updating a special method queues a slot update for every
329-
# affected subclass. Keep enough subclasses alive to require
330-
# heap-allocated queue chunks in addition to the stack chunk.
347+
# gh-155978: Updating a special method updates the slot of every
348+
# affected subclass.
331349
class Base:
332350
pass
333351

@@ -344,6 +362,11 @@ def custom_repr(self):
344362
self.assertTrue(all(repr(cls()) != "custom repr"
345363
for cls in subclasses))
346364

365+
# The slot update must not keep references to the types.
366+
del subclasses
367+
gc.collect()
368+
self.assertEqual(Base.__subclasses__(), [])
369+
347370
def test_concurrent_setattr_deadlock(self):
348371
# gh-155400: two threads assigning to a special method of the same
349372
# class could deadlock. One thread held the type lock and waited for
@@ -401,6 +424,151 @@ class B(A):
401424
with threading_helper.start_threads(threads):
402425
pass
403426

427+
def check_stop_the_world_under_type_lock(self, setter):
428+
# gh-158195: Python code that runs with the type lock held and stops
429+
# the world could deadlock with a thread that held the type lock
430+
# pinned while it waited for the stop-the-world mutex (__bases__
431+
# assignment). The first thread released the type lock while it was
432+
# blocked on the stop-the-world mutex, was handed the mutex, and then
433+
# waited for the type lock while owning the mutex.
434+
def basesetter():
435+
class A:
436+
pass
437+
class B:
438+
pass
439+
class C(A):
440+
pass
441+
for _ in range(100):
442+
C.__bases__ = (B,)
443+
C.__bases__ = (A,)
444+
445+
def stopper():
446+
for _ in range(100):
447+
_testinternalcapi.stop_the_world()
448+
449+
# The stopper keeps the stop-the-world mutex contended, which makes
450+
# the setters block on it. All workers have finite workloads so
451+
# that repeated invalidation cannot indefinitely delay a setter.
452+
threading_helper.run_concurrently(
453+
[setter, setter, basesetter, stopper])
454+
455+
def test_stop_the_world_in_mro_deadlock(self):
456+
# type_ready() calls a metaclass mro() with the type lock held.
457+
N = 50
458+
459+
class Meta(type):
460+
def mro(cls):
461+
_testinternalcapi.stop_the_world()
462+
return type.mro(cls)
463+
464+
def setter():
465+
for _ in range(N):
466+
Meta("K", (), {})
467+
468+
self.check_stop_the_world_under_type_lock(setter)
469+
470+
def test_reentrant_mro_always_publishes(self):
471+
# A custom mro() that updates a slot on every call must not make
472+
# the __bases__ assignment retry forever.
473+
class Other:
474+
pass
475+
476+
class Meta(type):
477+
def mro(cls):
478+
Other.__repr__ = lambda self: "x"
479+
return type.mro(cls)
480+
481+
class A:
482+
pass
483+
484+
class B:
485+
pass
486+
487+
class C(A, metaclass=Meta):
488+
pass
489+
490+
C.__bases__ = (B,)
491+
self.assertEqual(C.__mro__, (C, B, object))
492+
493+
def test_slot_update_skips_non_str_keys(self):
494+
# Slots are computed (with the world stopped, for an update) without
495+
# comparing the slot name with non-str keys of the type dicts, which
496+
# could run user code. Here, that code would also update a slot
497+
# every time.
498+
calls = 0
499+
500+
class Other:
501+
pass
502+
503+
class Key:
504+
def __hash__(self):
505+
return hash("__hash__")
506+
507+
def __eq__(self, other):
508+
nonlocal calls
509+
calls += 1
510+
Other.__repr__ = lambda self: "x"
511+
return False
512+
513+
class Base:
514+
pass
515+
516+
with self.assertWarns(RuntimeWarning):
517+
Sub = type("Sub", (Base,), {Key(): None})
518+
self.assertEqual(calls, 0)
519+
520+
Base.__hash__ = None
521+
self.assertEqual(calls, 0)
522+
with self.assertRaises(TypeError):
523+
hash(Sub())
524+
del Base.__hash__
525+
hash(Sub())
526+
527+
def test_concurrent_bases_assignment(self):
528+
# Several threads reassign __bases__ of the same class, and of its
529+
# base, while others stop the world. The type lock is released while
530+
# waiting for the stop-the-world. At the end, the MRO must agree
531+
# with __bases__.
532+
N = 50
533+
534+
class A:
535+
def meth(self):
536+
return "A"
537+
def __repr__(self):
538+
return "A"
539+
class B:
540+
def meth(self):
541+
return "B"
542+
def __repr__(self):
543+
return "B"
544+
class C(A):
545+
pass
546+
class D(C):
547+
pass
548+
549+
def setter():
550+
for _ in range(N):
551+
C.__bases__ = (B,)
552+
C.__bases__ = (A,)
553+
554+
def subsetter():
555+
for _ in range(N):
556+
D.__bases__ = (C, B)
557+
D.__bases__ = (C,)
558+
559+
threading_helper.run_concurrently(
560+
[setter, setter, subsetter] + world_stoppers())
561+
562+
self.assertEqual(C.__bases__, (A,))
563+
self.assertEqual(C.__mro__, (C, A, object))
564+
self.assertEqual(D.__bases__, (C,))
565+
self.assertEqual(D.__mro__, (D, C, A, object))
566+
self.assertEqual(D().meth(), "A")
567+
self.assertEqual(repr(C()), "A")
568+
self.assertEqual(repr(D()), "A")
569+
self.assertIn(C, A.__subclasses__())
570+
self.assertNotIn(C, B.__subclasses__())
571+
404572
def test_class_creation_with_concurrent_setattr(self):
405573
# Creating a class resolves its slots from the dicts along the MRO
406574
# while holding the type lock, before the class is reachable from
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Fix a deadlock in the free-threaded build between a thread that stops the
2+
world while running Python code under the type lock (for example a custom
3+
metaclass ``mro()`` or the ``__eq__`` of a :class:`str` subclass used as a
4+
class ``__dict__`` key) and a thread that assigns to a special method or to
5+
``__bases__`` of a class. The type lock is no longer held while waiting
6+
for a stop-the-world; the prepared slot, MRO and bases updates are
7+
validated with the world stopped and recomputed if they went stale.

‎Modules/_testinternalcapi.c‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
#include "pycore_pylifecycle.h" // _PyInterpreterConfig_InitFromDict()
3939
#include "pycore_pystate.h" // _PyThreadState_GET()
4040
#include "pycore_runtime_structs.h" // _PY_NSMALLPOSINTS
41+
#include "pycore_time.h" // _PyTime_FromSecondsObject()
4142
#include "pycore_unicodeobject.h" // _PyUnicode_TransformDecimalAndSpaceToASCII()
4243

4344
#include "clinic/_testinternalcapi.c.h"
@@ -209,6 +210,42 @@ get_stack_margin(PyObject *self, PyObject *Py_UNUSED(args))
209210
return PyLong_FromSize_t(_PyOS_STACK_MARGIN_BYTES);
210211
}
211212

213+
static PyObject *
214+
stop_the_world(PyObject *self, PyObject *args)
215+
{
216+
PyObject *duration_obj = NULL;
217+
PyTime_t duration = 0;
218+
if (!PyArg_ParseTuple(args, "|O:stop_the_world", &duration_obj)) {
219+
return NULL;
220+
}
221+
if (duration_obj != NULL &&
222+
_PyTime_FromSecondsObject(&duration, duration_obj,
223+
_PyTime_ROUND_TIMEOUT) < 0) {
224+
return NULL;
225+
}
226+
if (duration < 0) {
227+
PyErr_SetString(PyExc_ValueError, "duration must be non-negative");
228+
return NULL;
229+
}
230+
#ifdef Py_GIL_DISABLED
231+
PyInterpreterState *interp = _PyInterpreterState_GET();
232+
_PyEval_StopTheWorld(interp);
233+
if (duration != 0) {
234+
PyEvent event = {0};
235+
PyEvent_WaitTimed(&event, duration, /*detach=*/0);
236+
}
237+
_PyEval_StartTheWorld(interp);
238+
#endif
239+
Py_RETURN_NONE;
240+
}
241+
242+
PyDoc_STRVAR(stop_the_world_doc,
243+
"stop_the_world($module, duration=0, /)\n"
244+
"--\n"
245+
"\n"
246+
"Stop the world, wait for duration seconds, then restart it.\n"
247+
"Does nothing in GIL-enabled builds.");
248+
212249
static PyObject *
213250
test_stop_the_world(PyObject *self, PyObject *Py_UNUSED(args))
214251
{
@@ -3363,6 +3400,7 @@ static PyMethodDef module_functions[] = {
33633400
{"get_c_recursion_remaining", get_c_recursion_remaining, METH_NOARGS},
33643401
{"get_stack_pointer", get_stack_pointer, METH_NOARGS},
33653402
{"get_stack_margin", get_stack_margin, METH_NOARGS},
3403+
{"stop_the_world", stop_the_world, METH_VARARGS, stop_the_world_doc},
33663404
{"test_stop_the_world", test_stop_the_world, METH_NOARGS},
33673405
{"classify_stack_addresses", classify_stack_addresses, METH_VARARGS},
33683406
{"get_jit_code_ranges", get_jit_code_ranges, METH_NOARGS},

0 commit comments

Comments
 (0)