Skip to content

Commit 2fbc986

Browse files
colesburycolesbury
authored
[3.13] gh-128759: Fix accesses to tp_version_tag. (GH-129750) (GH-130042)
We should use a relaxed atomic load in the free threading build in `PyType_Modified()` because that's called without the type lock held. It's not necessary to use atomics in `type_modified_unlocked()`. We should also use `FT_ATOMIC_STORE_UINT_RELAXED()` instead of the `UINT32` variant because `tp_version_tag` is declared as `unsigned int`. (cherry picked from commit 57f45ee)
1 parent 4cb251d commit 2fbc986

File tree

2 files changed

+75
-9
lines changed

2 files changed

+75
-9
lines changed

‎Include/internal/pycore_pyatomic_ft_wrappers.h

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,61 @@ extern "C" {
6161
_Py_atomic_store_uint16_relaxed(&value, new_value)
6262
#define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) \
6363
_Py_atomic_store_uint32_relaxed(&value, new_value)
64+
#define FT_ATOMIC_STORE_CHAR_RELAXED(value, new_value) \
65+
_Py_atomic_store_char_relaxed(&value, new_value)
66+
#define FT_ATOMIC_LOAD_CHAR_RELAXED(value) \
67+
_Py_atomic_load_char_relaxed(&value)
68+
#define FT_ATOMIC_STORE_UCHAR_RELAXED(value, new_value) \
69+
_Py_atomic_store_uchar_relaxed(&value, new_value)
70+
#define FT_ATOMIC_LOAD_UCHAR_RELAXED(value) \
71+
_Py_atomic_load_uchar_relaxed(&value)
72+
#define FT_ATOMIC_STORE_SHORT_RELAXED(value, new_value) \
73+
_Py_atomic_store_short_relaxed(&value, new_value)
74+
#define FT_ATOMIC_LOAD_SHORT_RELAXED(value) \
75+
_Py_atomic_load_short_relaxed(&value)
76+
#define FT_ATOMIC_STORE_USHORT_RELAXED(value, new_value) \
77+
_Py_atomic_store_ushort_relaxed(&value, new_value)
78+
#define FT_ATOMIC_LOAD_USHORT_RELAXED(value) \
79+
_Py_atomic_load_ushort_relaxed(&value)
80+
#define FT_ATOMIC_STORE_INT_RELAXED(value, new_value) \
81+
_Py_atomic_store_int_relaxed(&value, new_value)
82+
#define FT_ATOMIC_LOAD_INT_RELAXED(value) \
83+
_Py_atomic_load_int_relaxed(&value)
84+
#define FT_ATOMIC_STORE_UINT_RELAXED(value, new_value) \
85+
_Py_atomic_store_uint_relaxed(&value, new_value)
86+
#define FT_ATOMIC_LOAD_UINT_RELAXED(value) \
87+
_Py_atomic_load_uint_relaxed(&value)
88+
#define FT_ATOMIC_STORE_LONG_RELAXED(value, new_value) \
89+
_Py_atomic_store_long_relaxed(&value, new_value)
90+
#define FT_ATOMIC_LOAD_LONG_RELAXED(value) \
91+
_Py_atomic_load_long_relaxed(&value)
92+
#define FT_ATOMIC_STORE_ULONG_RELAXED(value, new_value) \
93+
_Py_atomic_store_ulong_relaxed(&value, new_value)
94+
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) \
95+
_Py_atomic_store_ssize_relaxed(&value, new_value)
96+
#define FT_ATOMIC_STORE_FLOAT_RELAXED(value, new_value) \
97+
_Py_atomic_store_float_relaxed(&value, new_value)
98+
#define FT_ATOMIC_LOAD_FLOAT_RELAXED(value) \
99+
_Py_atomic_load_float_relaxed(&value)
100+
#define FT_ATOMIC_STORE_DOUBLE_RELAXED(value, new_value) \
101+
_Py_atomic_store_double_relaxed(&value, new_value)
102+
#define FT_ATOMIC_LOAD_DOUBLE_RELAXED(value) \
103+
_Py_atomic_load_double_relaxed(&value)
104+
#define FT_ATOMIC_STORE_LLONG_RELAXED(value, new_value) \
105+
_Py_atomic_store_llong_relaxed(&value, new_value)
106+
#define FT_ATOMIC_LOAD_LLONG_RELAXED(value) \
107+
_Py_atomic_load_llong_relaxed(&value)
108+
#define FT_ATOMIC_STORE_ULLONG_RELAXED(value, new_value) \
109+
_Py_atomic_store_ullong_relaxed(&value, new_value)
110+
#define FT_ATOMIC_LOAD_ULLONG_RELAXED(value) \
111+
_Py_atomic_load_ullong_relaxed(&value)
64112

65113
#else
66114
#define FT_ATOMIC_LOAD_PTR(value) value
67115
#define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value
68116
#define FT_ATOMIC_LOAD_SSIZE(value) value
69117
#define FT_ATOMIC_LOAD_SSIZE_ACQUIRE(value) value
70118
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value
71-
#define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value
72119
#define FT_ATOMIC_LOAD_PTR_ACQUIRE(value) value
73120
#define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) value
74121
#define FT_ATOMIC_LOAD_PTR_RELAXED(value) value
@@ -85,6 +132,30 @@ extern "C" {
85132
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value
86133
#define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value
87134
#define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) value = new_value
135+
#define FT_ATOMIC_LOAD_CHAR_RELAXED(value) value
136+
#define FT_ATOMIC_STORE_CHAR_RELAXED(value, new_value) value = new_value
137+
#define FT_ATOMIC_LOAD_UCHAR_RELAXED(value) value
138+
#define FT_ATOMIC_STORE_UCHAR_RELAXED(value, new_value) value = new_value
139+
#define FT_ATOMIC_LOAD_SHORT_RELAXED(value) value
140+
#define FT_ATOMIC_STORE_SHORT_RELAXED(value, new_value) value = new_value
141+
#define FT_ATOMIC_LOAD_USHORT_RELAXED(value) value
142+
#define FT_ATOMIC_STORE_USHORT_RELAXED(value, new_value) value = new_value
143+
#define FT_ATOMIC_LOAD_INT_RELAXED(value) value
144+
#define FT_ATOMIC_STORE_INT_RELAXED(value, new_value) value = new_value
145+
#define FT_ATOMIC_LOAD_UINT_RELAXED(value) value
146+
#define FT_ATOMIC_STORE_UINT_RELAXED(value, new_value) value = new_value
147+
#define FT_ATOMIC_LOAD_LONG_RELAXED(value) value
148+
#define FT_ATOMIC_STORE_LONG_RELAXED(value, new_value) value = new_value
149+
#define FT_ATOMIC_STORE_ULONG_RELAXED(value, new_value) value = new_value
150+
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value
151+
#define FT_ATOMIC_LOAD_FLOAT_RELAXED(value) value
152+
#define FT_ATOMIC_STORE_FLOAT_RELAXED(value, new_value) value = new_value
153+
#define FT_ATOMIC_LOAD_DOUBLE_RELAXED(value) value
154+
#define FT_ATOMIC_STORE_DOUBLE_RELAXED(value, new_value) value = new_value
155+
#define FT_ATOMIC_LOAD_LLONG_RELAXED(value) value
156+
#define FT_ATOMIC_STORE_LLONG_RELAXED(value, new_value) value = new_value
157+
#define FT_ATOMIC_LOAD_ULLONG_RELAXED(value) value
158+
#define FT_ATOMIC_STORE_ULLONG_RELAXED(value, new_value) value = new_value
88159

89160
#endif
90161

‎Objects/typeobject.c

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -975,7 +975,7 @@ set_version_unlocked(PyTypeObject *tp, unsigned int version)
975975
_Py_atomic_add_uint16(&tp->tp_versions_used, 1);
976976
}
977977
#endif
978-
FT_ATOMIC_STORE_UINT32_RELAXED(tp->tp_version_tag, version);
978+
FT_ATOMIC_STORE_UINT_RELAXED(tp->tp_version_tag, version);
979979
}
980980

981981
static void
@@ -996,15 +996,10 @@ type_modified_unlocked(PyTypeObject *type)
996996
We don't assign new version tags eagerly, but only as
997997
needed.
998998
*/
999-
#ifdef Py_GIL_DISABLED
1000-
if (_Py_atomic_load_uint_relaxed(&type->tp_version_tag) == 0) {
1001-
return;
1002-
}
1003-
#else
999+
ASSERT_TYPE_LOCK_HELD();
10041000
if (type->tp_version_tag == 0) {
10051001
return;
10061002
}
1007-
#endif
10081003
// Cannot modify static builtin types.
10091004
assert((type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) == 0);
10101005

@@ -1056,7 +1051,7 @@ void
10561051
PyType_Modified(PyTypeObject *type)
10571052
{
10581053
// Quick check without the lock held
1059-
if (type->tp_version_tag == 0) {
1054+
if (FT_ATOMIC_LOAD_UINT_RELAXED(type->tp_version_tag) == 0) {
10601055
return;
10611056
}
10621057

0 commit comments

Comments
 (0)