Skip to content

Commit 8475fd6

Browse files
mpagempage
committed
Make analyze_descriptor thread-safe
* Check that the type hasn't changed across lookups * Descr is now an owned ref
1 parent a576748 commit 8475fd6

File tree

1 file changed

+72
-17
lines changed

1 file changed

+72
-17
lines changed

‎Python/specialize.c‎

Lines changed: 72 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,7 @@ _PyCode_Quicken(_Py_CODEUNIT *instructions, Py_ssize_t size, PyObject *consts,
547547
#define SPEC_FAIL_ATTR_METACLASS_OVERRIDDEN 34
548548
#define SPEC_FAIL_ATTR_SPLIT_DICT 35
549549
#define SPEC_FAIL_ATTR_DESCR_NOT_DEFERRED 36
550+
#define SPEC_FAIL_ATTR_TYPE_CHANGED 37
550551

551552
/* Binary subscr and store subscr */
552553

@@ -834,7 +835,8 @@ typedef enum {
834835
ABSENT, /* Attribute is not present on the class */
835836
DUNDER_CLASS, /* __class__ attribute */
836837
GETSET_OVERRIDDEN, /* __getattribute__ or __setattr__ has been overridden */
837-
GETATTRIBUTE_IS_PYTHON_FUNCTION /* Descriptor requires calling a Python __getattribute__ */
838+
GETATTRIBUTE_IS_PYTHON_FUNCTION, /* Descriptor requires calling a Python __getattribute__ */
839+
TYPE_CHANGED, /* Error: the type changed during classification */
838840
} DescriptorClassification;
839841

840842

@@ -881,9 +883,11 @@ classify_descriptor(PyObject *descriptor, bool has_getattr)
881883
}
882884

883885
static DescriptorClassification
884-
analyze_descriptor(PyTypeObject *type, PyObject *name, PyObject **descr, int store)
886+
analyze_descriptor(PyTypeObject *type, PyObject *name, PyObject **descr, unsigned int *tp_version, int store)
885887
{
886888
bool has_getattr = false;
889+
bool have_getattribute_ver = false;
890+
unsigned int getattribute_ver;
887891
if (store) {
888892
if (type->tp_setattro != PyObject_GenericSetAttr) {
889893
*descr = NULL;
@@ -900,22 +904,34 @@ analyze_descriptor(PyTypeObject *type, PyObject *name, PyObject **descr, int sto
900904
getattro_slot == _Py_slot_tp_getattro) {
901905
/* One or both of __getattribute__ or __getattr__ may have been
902906
overridden See typeobject.c for why these functions are special. */
903-
PyObject *getattribute = _PyType_Lookup(type,
904-
&_Py_ID(__getattribute__));
907+
PyObject *getattribute = _PyType_LookupRefAndVersion(type,
908+
&_Py_ID(__getattribute__), &getattribute_ver);
909+
have_getattribute_ver = true;
905910
PyInterpreterState *interp = _PyInterpreterState_GET();
906911
bool has_custom_getattribute = getattribute != NULL &&
907912
getattribute != interp->callable_cache.object__getattribute__;
908-
has_getattr = _PyType_Lookup(type, &_Py_ID(__getattr__)) != NULL;
913+
unsigned int getattr_ver;
914+
PyObject *getattr = _PyType_LookupRefAndVersion(type, &_Py_ID(__getattr__), &getattr_ver);
915+
has_getattr = getattr != NULL;
916+
if (getattr_ver != getattribute_ver) {
917+
Py_XDECREF(getattribute);
918+
Py_XDECREF(getattr);
919+
return TYPE_CHANGED;
920+
}
909921
if (has_custom_getattribute) {
910922
if (getattro_slot == _Py_slot_tp_getattro &&
911923
!has_getattr &&
912924
Py_IS_TYPE(getattribute, &PyFunction_Type)) {
913925
*descr = getattribute;
926+
assert(getattr == NULL);
927+
*tp_version = getattribute_ver;
914928
return GETATTRIBUTE_IS_PYTHON_FUNCTION;
915929
}
916930
/* Potentially both __getattr__ and __getattribute__ are set.
917931
Too complicated */
918932
*descr = NULL;
933+
Py_XDECREF(getattribute);
934+
Py_XDECREF(getattr);
919935
return GETSET_OVERRIDDEN;
920936
}
921937
/* Potentially has __getattr__ but no custom __getattribute__.
@@ -925,14 +941,22 @@ analyze_descriptor(PyTypeObject *type, PyObject *name, PyObject **descr, int sto
925941
raised. This means some specializations, e.g. specializing
926942
for property() isn't safe.
927943
*/
944+
Py_XDECREF(getattr);
945+
Py_XDECREF(getattribute);
928946
}
929947
else {
930948
*descr = NULL;
931949
return GETSET_OVERRIDDEN;
932950
}
933951
}
934-
PyObject *descriptor = _PyType_Lookup(type, name);
952+
unsigned int descr_version;
953+
PyObject *descriptor = _PyType_LookupRefAndVersion(type, name, &descr_version);
954+
if (have_getattribute_ver && (descr_version != getattribute_ver)) {
955+
Py_XDECREF(descriptor);
956+
return TYPE_CHANGED;
957+
}
935958
*descr = descriptor;
959+
*tp_version = descr_version;
936960
if (PyUnicode_CompareWithASCIIString(name, "__class__") == 0) {
937961
if (descriptor == _PyType_Lookup(&PyBaseObject_Type, name)) {
938962
return DUNDER_CLASS;
@@ -1062,20 +1086,20 @@ instance_has_key(PyObject *obj, PyObject *name, uint32_t *shared_keys_version)
10621086
#endif
10631087

10641088
static int
1065-
specialize_instance_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject* name)
1089+
do_specialize_instance_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject* name,
1090+
bool shadow, uint32_t shared_keys_version,
1091+
DescriptorClassification kind, PyObject *descr, unsigned int tp_version)
10661092
{
10671093
_PyAttrCache *cache = (_PyAttrCache *)(instr + 1);
10681094
PyTypeObject *type = Py_TYPE(owner);
1069-
// 0 is not a valid version
1070-
uint32_t shared_keys_version = 0;
1071-
bool shadow = instance_has_key(owner, name, &shared_keys_version);
1072-
PyObject *descr = NULL;
1073-
DescriptorClassification kind = analyze_descriptor(type, name, &descr, 0);
1074-
assert(descr != NULL || kind == ABSENT || kind == GETSET_OVERRIDDEN);
1075-
if (type_get_version(type, LOAD_ATTR) == 0) {
1095+
if (tp_version == 0) {
1096+
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OUT_OF_VERSIONS);
10761097
return -1;
10771098
}
10781099
switch(kind) {
1100+
case TYPE_CHANGED:
1101+
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_ATTR_TYPE_CHANGED);
1102+
return -1;
10791103
case OVERRIDING:
10801104
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_ATTR_OVERRIDING_DESCRIPTOR);
10811105
return -1;
@@ -1245,6 +1269,21 @@ specialize_instance_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject* na
12451269

12461270
#undef FT_UNIMPLEMENTED
12471271

1272+
static int
1273+
specialize_instance_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject* name)
1274+
{
1275+
// 0 is not a valid version
1276+
uint32_t shared_keys_version = 0;
1277+
bool shadow = instance_has_key(owner, name, &shared_keys_version);
1278+
PyObject *descr = NULL;
1279+
unsigned int tp_version;
1280+
PyTypeObject *type = Py_TYPE(owner);
1281+
DescriptorClassification kind = analyze_descriptor(type, name, &descr, &tp_version, 0);
1282+
int result = do_specialize_instance_load_attr(owner, instr, name, shadow, shared_keys_version, kind, descr, tp_version);
1283+
Py_XDECREF(descr);
1284+
return result;
1285+
}
1286+
12481287
void
12491288
_Py_Specialize_LoadAttr(_PyStackRef owner_st, _Py_CODEUNIT *instr, PyObject *name)
12501289
{
@@ -1290,6 +1329,7 @@ _Py_Specialize_StoreAttr(_PyStackRef owner_st, _Py_CODEUNIT *instr, PyObject *na
12901329
assert(_PyOpcode_Caches[STORE_ATTR] == INLINE_CACHE_ENTRIES_STORE_ATTR);
12911330
_PyAttrCache *cache = (_PyAttrCache *)(instr + 1);
12921331
PyTypeObject *type = Py_TYPE(owner);
1332+
PyObject *descr = NULL;
12931333
if (!_PyType_IsReady(type)) {
12941334
// We *might* not really need this check, but we inherited it from
12951335
// PyObject_GenericSetAttr and friends... and this way we still do the
@@ -1301,12 +1341,15 @@ _Py_Specialize_StoreAttr(_PyStackRef owner_st, _Py_CODEUNIT *instr, PyObject *na
13011341
SPECIALIZATION_FAIL(STORE_ATTR, SPEC_FAIL_OVERRIDDEN);
13021342
goto fail;
13031343
}
1304-
PyObject *descr;
1305-
DescriptorClassification kind = analyze_descriptor(type, name, &descr, 1);
1344+
unsigned int tp_version;
1345+
DescriptorClassification kind = analyze_descriptor(type, name, &descr, &tp_version, 1);
13061346
if (type_get_version(type, STORE_ATTR) == 0) {
13071347
goto fail;
13081348
}
13091349
switch(kind) {
1350+
case TYPE_CHANGED:
1351+
SPECIALIZATION_FAIL(STORE_ATTR, SPEC_FAIL_ATTR_TYPE_CHANGED);
1352+
goto fail;
13101353
case OVERRIDING:
13111354
SPECIALIZATION_FAIL(STORE_ATTR, SPEC_FAIL_ATTR_OVERRIDING_DESCRIPTOR);
13121355
goto fail;
@@ -1375,11 +1418,13 @@ _Py_Specialize_StoreAttr(_PyStackRef owner_st, _Py_CODEUNIT *instr, PyObject *na
13751418
assert(!PyErr_Occurred());
13761419
instr->op.code = STORE_ATTR;
13771420
cache->counter = adaptive_counter_backoff(cache->counter);
1421+
Py_XDECREF(descr);
13781422
return;
13791423
success:
13801424
STAT_INC(STORE_ATTR, success);
13811425
assert(!PyErr_Occurred());
13821426
cache->counter = adaptive_counter_cooldown();
1427+
Py_XDECREF(descr);
13831428
}
13841429

13851430
#ifndef Py_GIL_DISABLED
@@ -1448,18 +1493,25 @@ specialize_class_load_attr(PyObject *owner, _Py_CODEUNIT *instr,
14481493
}
14491494
PyObject *descr = NULL;
14501495
DescriptorClassification kind = 0;
1451-
kind = analyze_descriptor(cls, name, &descr, 0);
1496+
unsigned int tp_version;
1497+
kind = analyze_descriptor(cls, name, &descr, &tp_version, 0);
14521498
if (type_get_version(cls, LOAD_ATTR) == 0) {
1499+
Py_XDECREF(descr);
14531500
return -1;
14541501
}
14551502
bool metaclass_check = false;
14561503
if ((Py_TYPE(cls)->tp_flags & Py_TPFLAGS_IMMUTABLETYPE) == 0) {
14571504
metaclass_check = true;
14581505
if (type_get_version(Py_TYPE(cls), LOAD_ATTR) == 0) {
1506+
Py_XDECREF(descr);
14591507
return -1;
14601508
}
14611509
}
14621510
switch (kind) {
1511+
case TYPE_CHANGED:
1512+
SPECIALIZATION_FAIL(STORE_ATTR, SPEC_FAIL_ATTR_TYPE_CHANGED);
1513+
Py_XDECREF(descr);
1514+
return -1;
14631515
case METHOD:
14641516
case NON_DESCRIPTOR:
14651517
write_u32(cache->type_version, cls->tp_version_tag);
@@ -1471,14 +1523,17 @@ specialize_class_load_attr(PyObject *owner, _Py_CODEUNIT *instr,
14711523
else {
14721524
specialize(instr, LOAD_ATTR_CLASS);
14731525
}
1526+
Py_XDECREF(descr);
14741527
return 0;
14751528
#ifdef Py_STATS
14761529
case ABSENT:
14771530
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_EXPECTED_ERROR);
1531+
Py_XDECREF(descr);
14781532
return -1;
14791533
#endif
14801534
default:
14811535
SPECIALIZATION_FAIL(LOAD_ATTR, load_attr_fail_kind(kind));
1536+
Py_XDECREF(descr);
14821537
return -1;
14831538
}
14841539
}

0 commit comments

Comments
 (0)