gh-127266: avoid data races when updating type slots (gh-133177)

In the free-threaded build, avoid data races caused by updating type
slots or type flags after the type was initially created.  For those
(typically rare) cases, use the stop-the-world mechanism.  Remove the
use of atomics when reading or writing type flags.
This commit is contained in:
Neil Schemenauer 2025-05-27 18:27:41 -07:00 committed by GitHub
parent 7ca6d79fa3
commit fbbbc10055
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 575 additions and 133 deletions

View file

@ -677,8 +677,11 @@ struct _Py_interp_cached_objects {
/* object.__reduce__ */
PyObject *objreduce;
#ifndef Py_GIL_DISABLED
/* resolve_slotdups() */
PyObject *type_slots_pname;
pytype_slotdef *type_slots_ptrs[MAX_EQUIV];
#endif
/* TypeVar and related types */
PyTypeObject *generic_type;

View file

@ -313,7 +313,7 @@ extern int _PyDict_CheckConsistency(PyObject *mp, int check_content);
// Fast inlined version of PyType_HasFeature()
static inline int
_PyType_HasFeature(PyTypeObject *type, unsigned long feature) {
return ((FT_ATOMIC_LOAD_ULONG_RELAXED(type->tp_flags) & feature) != 0);
return ((type->tp_flags) & feature) != 0;
}
extern void _PyType_InitCache(PyInterpreterState *interp);

View file

@ -134,7 +134,6 @@ extern int _PyType_AddMethod(PyTypeObject *, PyMethodDef *);
extern void _PyType_SetFlagsRecursive(PyTypeObject *self, unsigned long mask,
unsigned long flags);
extern unsigned int _PyType_GetVersionForCurrentState(PyTypeObject *tp);
PyAPI_FUNC(void) _PyType_SetVersion(PyTypeObject *tp, unsigned int version);
PyTypeObject *_PyType_LookupByVersion(unsigned int version);

View file

@ -620,6 +620,12 @@ given type object has a specified feature.
#define Py_TPFLAGS_HAVE_FINALIZE (1UL << 0)
#define Py_TPFLAGS_HAVE_VERSION_TAG (1UL << 18)
// Flag values for ob_flags (16 bits available, if SIZEOF_VOID_P > 4).
#define _Py_IMMORTAL_FLAGS (1 << 0)
#define _Py_STATICALLY_ALLOCATED_FLAG (1 << 2)
#if defined(Py_GIL_DISABLED) && defined(Py_DEBUG)
#define _Py_TYPE_REVEALED_FLAG (1 << 3)
#endif
#define Py_CONSTANT_NONE 0
#define Py_CONSTANT_FALSE 1
@ -776,11 +782,7 @@ PyType_HasFeature(PyTypeObject *type, unsigned long feature)
// PyTypeObject is opaque in the limited C API
flags = PyType_GetFlags(type);
#else
# ifdef Py_GIL_DISABLED
flags = _Py_atomic_load_ulong_relaxed(&type->tp_flags);
# else
flags = type->tp_flags;
# endif
#endif
return ((flags & feature) != 0);
}

View file

@ -19,9 +19,6 @@ immortal. The latter should be the only instances that require
cleanup during runtime finalization.
*/
#define _Py_STATICALLY_ALLOCATED_FLAG 4
#define _Py_IMMORTAL_FLAGS 1
#if SIZEOF_VOID_P > 4
/*
In 64+ bit systems, any object whose 32 bit reference count is >= 2**31

View file

@ -4114,6 +4114,34 @@ class ClassPropertiesAndMethods(unittest.TestCase):
else:
self.fail("shouldn't be able to create inheritance cycles")
def test_assign_bases_many_subclasses(self):
# This is intended to check that typeobject.c:queue_slot_update() can
# handle updating many subclasses when a slot method is re-assigned.
class A:
x = 'hello'
def __call__(self):
return 123
def __getitem__(self, index):
return None
class X:
x = 'bye'
class B(A):
pass
subclasses = []
for i in range(1000):
sc = type(f'Sub{i}', (B,), {})
subclasses.append(sc)
self.assertEqual(subclasses[0]()(), 123)
self.assertEqual(subclasses[0]().x, 'hello')
B.__bases__ = (X,)
with self.assertRaises(TypeError):
subclasses[0]()()
self.assertEqual(subclasses[0]().x, 'bye')
def test_builtin_bases(self):
# Make sure all the builtin types can have their base queried without
# segfaulting. See issue #5787.

View file

@ -0,0 +1,6 @@
In the free-threaded build, avoid data races caused by updating type slots
or type flags after the type was initially created. For those (typically
rare) cases, use the stop-the-world mechanism. Remove the use of atomics
when reading or writing type flags. The use of atomics is not sufficient to
avoid races (since flags are sometimes read without a lock and without
atomics) and are no longer required.

File diff suppressed because it is too large Load diff

View file

@ -139,6 +139,19 @@
#endif
static void
check_invalid_reentrancy(void)
{
#if defined(Py_DEBUG) && defined(Py_GIL_DISABLED)
// In the free-threaded build, the interpreter must not be re-entered if
// the world-is-stopped. If so, that's a bug somewhere (quite likely in
// the painfully complex typeobject code).
PyInterpreterState *interp = _PyInterpreterState_GET();
assert(!interp->stoptheworld.world_stopped);
#endif
}
#ifdef Py_DEBUG
static void
dump_item(_PyStackRef item)
@ -996,6 +1009,7 @@ PyObject* _Py_HOT_FUNCTION DONT_SLP_VECTORIZE
_PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int throwflag)
{
_Py_EnsureTstateNotNULL(tstate);
check_invalid_reentrancy();
CALL_STAT_INC(pyeval_calls);
#if USE_COMPUTED_GOTOS && !Py_TAIL_CALL_INTERP

View file

@ -12,15 +12,12 @@
# These warnings trigger directly in a CPython function.
race_top:assign_version_tag
race_top:_Py_slot_tp_getattr_hook
race_top:dump_traceback
race_top:fatal_error
race_top:_PyFrame_GetCode
race_top:_PyFrame_Initialize
race_top:_PyObject_TryGetInstanceAttribute
race_top:PyUnstable_InterpreterFrame_GetLine
race_top:type_modified_unlocked
race_top:write_thread_id
# gh-129068: race on shared range iterators (test_free_threading.test_zip.ZipThreading.test_threading)
@ -29,9 +26,6 @@ race_top:rangeiter_next
# gh-129748: test.test_free_threading.test_slots.TestSlots.test_object
race_top:mi_block_set_nextx
# gh-127266: type slot updates are not thread-safe (test_opcache.test_load_attr_method_lazy_dict)
race_top:update_one_slot
# https://gist.github.com/mpage/6962e8870606cfc960e159b407a0cb40
thread:pthread_create
@ -49,7 +43,6 @@ race:list_inplace_repeat_lock_held
race:PyObject_Realloc
# gh-133467. Some of these could be hard to trigger.
race_top:update_one_slot
race_top:_Py_slot_tp_getattr_hook
race_top:slot_tp_descr_get
race_top:type_set_name