Skip to content

Commit f6d482e

Browse files
committed
Fix some racing reads in typebobject.c
1 parent 345e1e0 commit f6d482e

File tree

2 files changed

+17
-6
lines changed

2 files changed

+17
-6
lines changed

Include/internal/pycore_pyatomic_ft_wrappers.h

+6
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ extern "C" {
3939
_Py_atomic_load_uint8(&value)
4040
#define FT_ATOMIC_STORE_UINT8(value, new_value) \
4141
_Py_atomic_store_uint8(&value, new_value)
42+
#define FT_ATOMIC_LOAD_UINT32_RELAXED(value) \
43+
_Py_atomic_load_uint32_relaxed(&value)
4244
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \
4345
_Py_atomic_store_ptr_relaxed(&value, new_value)
4446
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) \
@@ -49,6 +51,8 @@ extern "C" {
4951
_Py_atomic_store_ssize_relaxed(&value, new_value)
5052
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \
5153
_Py_atomic_store_uint8_relaxed(&value, new_value)
54+
#define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) \
55+
_Py_atomic_store_uint32_relaxed(&value, new_value)
5256

5357
#else
5458
#define FT_ATOMIC_LOAD_PTR(value) value
@@ -62,11 +66,13 @@ extern "C" {
6266
#define FT_ATOMIC_LOAD_PTR_RELAXED(value) value
6367
#define FT_ATOMIC_LOAD_UINT8(value) value
6468
#define FT_ATOMIC_STORE_UINT8(value, new_value) value = new_value
69+
#define FT_ATOMIC_LOAD_UINT32_RELAXED(value) value
6570
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value
6671
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value
6772
#define FT_ATOMIC_STORE_UINTPTR_RELEASE(value, new_value) value = new_value
6873
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value
6974
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value
75+
#define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) value = new_value
7076

7177
#endif
7278

Objects/typeobject.c

+11-6
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ class object "PyObject *" "&PyBaseObject_Type"
4343
& ((1 << MCACHE_SIZE_EXP) - 1))
4444

4545
#define MCACHE_HASH_METHOD(type, name) \
46-
MCACHE_HASH((type)->tp_version_tag, ((Py_ssize_t)(name)) >> 3)
46+
MCACHE_HASH(FT_ATOMIC_LOAD_UINT32_RELAXED((type)->tp_version_tag), \
47+
((Py_ssize_t)(name)) >> 3)
4748
#define MCACHE_CACHEABLE_NAME(name) \
4849
PyUnicode_CheckExact(name) && \
4950
PyUnicode_IS_READY(name) && \
@@ -907,7 +908,7 @@ type_modified_unlocked(PyTypeObject *type)
907908
}
908909

909910
type->tp_flags &= ~Py_TPFLAGS_VALID_VERSION_TAG;
910-
type->tp_version_tag = 0; /* 0 is not a valid version tag */
911+
FT_ATOMIC_STORE_UINT32_RELAXED(type->tp_version_tag, 0); /* 0 is not a valid version tag */
911912
if (PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) {
912913
// This field *must* be invalidated if the type is modified (see the
913914
// comment on struct _specialization_cache):
@@ -984,7 +985,7 @@ type_mro_modified(PyTypeObject *type, PyObject *bases) {
984985
clear:
985986
assert(!(type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN));
986987
type->tp_flags &= ~Py_TPFLAGS_VALID_VERSION_TAG;
987-
type->tp_version_tag = 0; /* 0 is not a valid version tag */
988+
FT_ATOMIC_STORE_UINT32_RELAXED(type->tp_version_tag, 0); /* 0 is not a valid version tag */
988989
if (PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) {
989990
// This field *must* be invalidated if the type is modified (see the
990991
// comment on struct _specialization_cache):
@@ -1020,7 +1021,8 @@ assign_version_tag(PyInterpreterState *interp, PyTypeObject *type)
10201021
/* We have run out of version numbers */
10211022
return 0;
10221023
}
1023-
type->tp_version_tag = NEXT_GLOBAL_VERSION_TAG++;
1024+
FT_ATOMIC_STORE_UINT32_RELAXED(type->tp_version_tag,
1025+
NEXT_GLOBAL_VERSION_TAG++);
10241026
assert (type->tp_version_tag <= _Py_MAX_GLOBAL_TYPE_VERSION_TAG);
10251027
}
10261028
else {
@@ -1029,7 +1031,8 @@ assign_version_tag(PyInterpreterState *interp, PyTypeObject *type)
10291031
/* We have run out of version numbers */
10301032
return 0;
10311033
}
1032-
type->tp_version_tag = NEXT_VERSION_TAG(interp)++;
1034+
FT_ATOMIC_STORE_UINT32_RELAXED(type->tp_version_tag,
1035+
NEXT_VERSION_TAG(interp)++);
10331036
assert (type->tp_version_tag != 0);
10341037
}
10351038

@@ -5047,7 +5050,9 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name)
50475050
// synchronize-with other writing threads by doing an acquire load on the sequence
50485051
while (1) {
50495052
int sequence = _PySeqLock_BeginRead(&entry->sequence);
5050-
if (_Py_atomic_load_uint32_relaxed(&entry->version) == type->tp_version_tag &&
5053+
uint32_t entry_version = _Py_atomic_load_uint32_relaxed(&entry->version);
5054+
uint32_t type_version = _Py_atomic_load_uint32_relaxed(&type->tp_version_tag);
5055+
if (entry_version == type_version &&
50515056
_Py_atomic_load_ptr_relaxed(&entry->name) == name) {
50525057
assert(_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG));
50535058
OBJECT_STAT_INC_COND(type_cache_hits, !is_dunder_name(name));

0 commit comments

Comments
 (0)