Naserume · GitHub

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

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,

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.

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);

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

Read the original on github.com ↗