Data races on type slots and `tp_subclasses` between `type_setattro` and `type_new` / `type_dealloc`

[Naserume]

Bug report

Bug description:

On the type_setattro path, update_one_slot first reads the slot in queue_slot_update, and then defers the slot write to a stop-the-world phase.

https://github.com/python/cpython/blob/b18168cb32d545ed976b760983478cbd5 dde5bdf/Objects/typeobject.c#L3859-L3863

but on the type_new path it writes the slot immediately, with no stop-the-world

cpython/Objects/typeobject.c

Lines 12032 to 12041 in b18168c

	slot_value = specific;
	} else {
	slot_value = generic;
	}
	#ifdef Py_GIL_DISABLED
	if (queued_updates != NULL) {
	// queue the update to perform later, while world is stopped
	if (queue_slot_update(queued_updates, type, ptr, slot_value) < 0) {
	return -1;

The same two paths also race on the base's tp_subclasses dict.

recurse_down_subclasses iterates it with PyDict_Next,

cpython/Objects/typeobject.c

Lines 12351 to 12366 in b18168c

	static int
	recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name,
	update_callback callback, void *data)
	{
	// It is safe to use a borrowed reference because update_subclasses() is
	// only used with update_slots_callback() which doesn't modify
	// tp_subclasses.
	PyObject *subclasses = lookup_tp_subclasses(type); // borrowed ref
	if (subclasses == NULL) {
	return 0;
	}
	assert(PyDict_CheckExact(subclasses));
	Py_ssize_t i = 0;
	PyObject *ref;
	while (PyDict_Next(subclasses, &i, NULL, &ref)) {

while add_subclass inserts and remove_subclass deletes with no coordinating lock.

cpython/Objects/typeobject.c

Lines 9700 to 9710 in b18168c

	subclasses = init_tp_subclasses(base);
	if (subclasses == NULL) {
	Py_DECREF(key);
	Py_DECREF(ref);
	return -1;
	}
	}
	assert(PyDict_CheckExact(subclasses));
	int result = PyDict_SetItem(subclasses, key, ref);
	Py_DECREF(ref);

cpython/Objects/typeobject.c

Lines 9762 to 9772 in b18168c

	static void
	remove_subclass(PyTypeObject *base, PyTypeObject *type)
	{
	PyObject *subclasses = lookup_tp_subclasses(base); // borrowed ref
	if (subclasses == NULL) {
	return;
	}
	assert(PyDict_CheckExact(subclasses));
	PyObject *key = get_subclasses_key(type, base);
	if (key != NULL && PyDict_DelItem(subclasses, key)) {

Reproducer:

import threading

class Base:
 pass

def setter():
 for _ in range(10000):
 Base.__repr__ = lambda self: "x"

def subclasser():
 for _ in range(10000):
 type('Sub', (Base,), {})

threads = [threading.Thread(target=setter) for _ in range(2)]
threads += [threading.Thread(target=subclasser) for _ in range(4)]
for t in threads: t.start()
for t in threads: t.join()

TSAN Report:

==================
WARNING: ThreadSanitizer: data race (pid=1674015)
 Read of size 8 at 0x7fffc2100468 by thread T2:
 #0 queue_slot_update /cpython/Objects/typeobject.c:3858:9 
 #1 update_one_slot /cpython/Objects/typeobject.c:12035:13 
 #2 update_slots_callback /cpython/Objects/typeobject.c:12064:13 
 #3 update_subclasses /cpython/Objects/typeobject.c:12340:9 
 #4 recurse_down_subclasses /cpython/Objects/typeobject.c:12381:13 
 #5 update_subclasses /cpython/Objects/typeobject.c:12343:12
 #6 update_slot /cpython/Objects/typeobject.c:12108:12 
 #7 update_slot_after_setattr /cpython/Objects/typeobject.c:6719:9 
 #8 type_setattro /cpython/Objects/typeobject.c:6820:19 
 #9 PyObject_SetAttr /cpython/Objects/object.c:1533:15 
 #10 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:11536:27 
...

 Previous write of size 8 at 0x7fffc2100468 by thread T4:
 #0 update_one_slot /cpython/Objects/typeobject.c:12040:14 
 #1 fixup_slot_dispatchers /cpython/Objects/typeobject.c:12120:9 
 #2 type_new_impl /cpython/Objects/typeobject.c:4952:5 
 #3 type_new /cpython/Objects/typeobject.c:5100:12 
 #4 type_call /cpython/Objects/typeobject.c:2467:11 
 #5 _PyObject_MakeTpCall /cpython/Objects/call.c:242:18 
 #6 type_vectorcall /cpython/Objects/typeobject.c:5120:12 
 #7 _Py_CallBuiltinClass_StackRef /cpython/Python/ceval.c:899:11 
 #8 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:2333:35 
...

SUMMARY: ThreadSanitizer: data race /cpython/Objects/typeobject.c:3858:9 in queue_slot_update
==================
==================
WARNING: ThreadSanitizer: data race (pid=1674015)
 Atomic write of size 8 at 0x7fffc62210f8 by thread T6:
 #0 delitem_common /cpython/Objects/dictobject.c 
 #1 _PyDict_DelItem_KnownHash_LockHeld /cpython/Objects/dictobject.c:2966:5 
 #2 _PyDict_DelItem_KnownHash /cpython/Objects/dictobject.c:2975:11 
 #3 PyDict_DelItem /cpython/Objects/dictobject.c:2934:12 
 #4 remove_subclass /cpython/Objects/typeobject.c:9767:24 
 #5 remove_all_subclasses /cpython/Objects/typeobject.c:9790:13 
 #6 type_dealloc_common /cpython/Objects/typeobject.c:6839:9 
 #7 type_dealloc /cpython/Objects/typeobject.c:6998:5 
 #8 _Py_Dealloc /cpython/Objects/object.c:3312:5 
 #9 _Py_DecRefSharedDebug /cpython/Objects/object.c:426:9 
 #10 _Py_DecRefShared /cpython/Objects/object.c:433:5 
 #11 Py_DECREF /cpython/./Include/refcount.h:385:9 
 #12 Py_XDECREF /cpython/./Include/refcount.h:520:9 
 #13 tuple_dealloc /cpython/Objects/tupleobject.c:277:9 
 #14 _Py_Dealloc /cpython/Objects/object.c:3312:5 
 #15 _Py_DecRefSharedDebug /cpython/Objects/object.c:426:9 
 #16 _Py_DecRefShared /cpython/Objects/object.c:433:5 
 #17 Py_DECREF /cpython/./Include/refcount.h:385:9 
 #18 delete_garbage /cpython/Python/gc_free_threading.c 
 #19 gc_collect_internal /cpython/Python/gc_free_threading.c:2176:5 
 #20 gc_collect_main /cpython/Python/gc_free_threading.c:2257:5 
 #21 _Py_RunGC /cpython/Python/gc_free_threading.c:2716:5 
 #22 _Py_HandlePending /cpython/Python/ceval_gil.c:1399:9 
 #23 check_periodics /cpython/Python/ceval_macros.h:524:16 
 #24 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:2367:27 
...

 Previous read of size 8 at 0x7fffc62210f8 by thread T1:
 #0 _PyDict_Next /cpython/Objects/dictobject.c:3163:31 
 #1 PyDict_Next /cpython/Objects/dictobject.c:3198:12 
 #2 recurse_down_subclasses /cpython/Objects/typeobject.c:12361:12 
 #3 update_subclasses /cpython/Objects/typeobject.c:12343:12 
 #4 update_slot /cpython/Objects/typeobject.c:12108:12
 #5 update_slot_after_setattr /cpython/Objects/typeobject.c:6719:9 
 #6 type_setattro /cpython/Objects/typeobject.c:6820:19 
 #7 PyObject_SetAttr /cpython/Objects/object.c:1533:15 
 #8 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:11536:27 
...

SUMMARY: ThreadSanitizer: data race /cpython/Objects/dictobject.c in delitem_common
==================

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

• gh-151377: Fix races updating type slots and subclasses in free-threa… #151394
• gh-151377: Fix races updating type slots and subclasses #155370

[kumaraditya303]

cc @nascheme