gh-137238: Fix data race in _Py_slot_tp_getattr_hook (gh-137240)

Replacing the slot isn't thread-safe if the GIL is disabled. Don't
require that the slot has been replaced when specializing.
This commit is contained in:
Sam Gross
2025-08-05 09:32:22 -04:00
committed by GitHub
parent a50822ff94
commit 485b16b4f7
3 changed files with 4 additions and 9 deletions

View File

@@ -10585,7 +10585,10 @@ _Py_slot_tp_getattr_hook(PyObject *self, PyObject *name)
getattr = _PyType_LookupRef(tp, &_Py_ID(__getattr__));
if (getattr == NULL) {
/* No __getattr__ hook: use a simpler dispatcher */
#ifndef Py_GIL_DISABLED
// Replacing the slot is only thread-safe if there is a GIL.
tp->tp_getattro = _Py_slot_tp_getattro;
#endif
return _Py_slot_tp_getattro(self, name);
}
/* speed hack: we could use lookup_maybe, but that would resolve the

View File

@@ -935,8 +935,7 @@ analyze_descriptor_load(PyTypeObject *type, PyObject *name, PyObject **descr, un
PyObject *getattr = _PyType_Lookup(type, &_Py_ID(__getattr__));
has_getattr = getattr != NULL;
if (has_custom_getattribute) {
if (getattro_slot == _Py_slot_tp_getattro &&
!has_getattr &&
if (!has_getattr &&
Py_IS_TYPE(getattribute, &PyFunction_Type)) {
*descr = getattribute;
*tp_version = ga_version;
@@ -1259,12 +1258,6 @@ do_specialize_instance_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject*
return -1;
case GETATTRIBUTE_IS_PYTHON_FUNCTION:
{
#ifndef Py_GIL_DISABLED
// In free-threaded builds it's possible for tp_getattro to change
// after the call to analyze_descriptor. That is fine: the version
// guard will fail.
assert(type->tp_getattro == _Py_slot_tp_getattro);
#endif
assert(Py_IS_TYPE(descr, &PyFunction_Type));
_PyLoadMethodCache *lm_cache = (_PyLoadMethodCache *)(instr + 1);
if (!function_check_args(descr, 2, LOAD_ATTR)) {

View File

@@ -43,6 +43,5 @@ race:list_inplace_repeat_lock_held
race:PyObject_Realloc
# gh-133467. Some of these could be hard to trigger.
race_top:_Py_slot_tp_getattr_hook
race_top:set_tp_bases
race_top:type_set_bases_unlocked