Skip to content

Commit 4fd24bc

Browse files
eendebakptclaude
andcommitted
gh-NNNNN: Make _STORE_ATTR_SLOT lock-free on the free-threaded build
Replace the LOCK_OBJECT / atomic-release-store / UNLOCK_OBJECT sequence in _STORE_ATTR_SLOT (and the matching critical section in PyMember_SetOne for Py_T_OBJECT_EX / _Py_T_OBJECT) with a single atomic exchange on the slot pointer. Atomic exchange returns a unique old value per writer, so Py_XDECREF cannot double-free across concurrent writers. Concurrent readers (_LOAD_ATTR_SLOT, PyMember_GetOne) already use atomic load + _Py_TryIncrefCompare; PyMember_GetOne's locked fallback is replaced by _Py_XGetRef so it also stays correct against lock-free writers. Drops HAS_DEOPT_FLAG from _STORE_ATTR_SLOT (no LOCK can fail anymore), which removes one _DEOPT exit per occurrence from Tier 2 traces. Adds _Py_atomic_exchange_ptr to the cases-generator's NON_ESCAPING_FUNCTIONS allowlist so the generator stops wrapping it in _PyFrame_SetStackPointer / _GetStackPointer. Adds a concurrent _Py_T_OBJECT test (test_T_OBJECT in test_slots.py). Benchmarks (PGO+LTO, taskset -c 4, pyperf): micro_store_attr_slot 79.0 ns -> 54.5 ns 1.45x faster micro_attr_idiv 180 ns -> 149 ns 1.21x faster bm_float 87.4 ms -> 83.7 ms 1.04x faster bm_nbody (control) unchanged Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9df2b6c commit 4fd24bc

9 files changed

Lines changed: 77 additions & 54 deletions

File tree

Include/internal/pycore_opcode_metadata.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Include/internal/pycore_uop_metadata.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Lib/test/test_free_threading/test_slots.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,3 +304,21 @@ def reader():
304304
spam_new.T_CHAR
305305

306306
run_in_threads([writer, reader])
307+
308+
def test_T_OBJECT(self):
309+
# _Py_T_OBJECT exercises PyMember_GetOne / PyMember_SetOne for
310+
# the object case from concurrent threads. Uses
311+
# _testcapi.HeapCTypeWithDict which exposes its `dict` slot as a
312+
# `_Py_T_OBJECT` member named `dictobj`.
313+
obj = _testcapi.HeapCTypeWithDict()
314+
315+
def writer():
316+
for i in range(1_000):
317+
obj.dictobj = {"k": i}
318+
319+
def reader():
320+
for _ in range(1_000):
321+
v = obj.dictobj
322+
assert v is None or isinstance(v, dict)
323+
324+
run_in_threads([writer, reader])

Modules/_testinternalcapi/test_cases.c.h

Lines changed: 6 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/bytecodes.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3225,14 +3225,21 @@ dummy_func(
32253225
op(_STORE_ATTR_SLOT, (index/1, value, owner -- o)) {
32263226
PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner);
32273227

3228-
DEOPT_IF(!LOCK_OBJECT(owner_o));
32293228
char *addr = (char *)owner_o + index;
32303229
STAT_INC(STORE_ATTR, hit);
3230+
// Atomic exchange of the slot pointer. Each concurrent writer
3231+
// observes a unique old value, so Py_XDECREF below cannot
3232+
// double-free. Concurrent readers (_LOAD_ATTR_SLOT,
3233+
// PyMember_GetOne) cope via _Py_TryIncrefCompare.
3234+
#ifdef Py_GIL_DISABLED
3235+
PyObject *old_value = _Py_atomic_exchange_ptr(
3236+
(void **)addr, PyStackRef_AsPyObjectSteal(value));
3237+
#else
32313238
PyObject *old_value = *(PyObject **)addr;
3232-
FT_ATOMIC_STORE_PTR_RELEASE(*(PyObject **)addr, PyStackRef_AsPyObjectSteal(value));
3233-
UNLOCK_OBJECT(owner_o);
3234-
INPUTS_DEAD();
3239+
*(PyObject **)addr = PyStackRef_AsPyObjectSteal(value);
3240+
#endif
32353241
o = owner;
3242+
DEAD(owner);
32363243
Py_XDECREF(old_value);
32373244
}
32383245

Python/executor_cases.c.h

Lines changed: 6 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/generated_cases.c.h

Lines changed: 6 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/structmember.c

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -97,36 +97,33 @@ PyMember_GetOne(const char *obj_addr, PyMemberDef *l)
9797
break;
9898
}
9999
case _Py_T_OBJECT:
100-
v = FT_ATOMIC_LOAD_PTR(*(PyObject **) addr);
101-
if (v != NULL) {
102100
#ifdef Py_GIL_DISABLED
103-
if (!_Py_TryIncrefCompare((PyObject **) addr, v)) {
104-
Py_BEGIN_CRITICAL_SECTION((PyObject *) obj_addr);
105-
v = FT_ATOMIC_LOAD_PTR(*(PyObject **) addr);
106-
Py_XINCREF(v);
107-
Py_END_CRITICAL_SECTION();
108-
}
101+
// _Py_XGetRef does an atomic load + try-incref loop, retrying when
102+
// a concurrent writer (lock-free atomic exchange in _STORE_ATTR_SLOT
103+
// and PyMember_SetOne) mutates the slot. The previous critical-
104+
// section fallback assumed writers also took ob_mutex, which they no
105+
// longer do, so a plain Py_XINCREF inside the CS could resurrect a
106+
// refcount-0 (about-to-be-freed) object.
107+
v = _Py_XGetRef((PyObject **)addr);
109108
#else
110-
Py_INCREF(v);
109+
v = *(PyObject **)addr;
110+
Py_XINCREF(v);
111111
#endif
112-
}
113112
if (v == NULL) {
114-
v = Py_None;
113+
v = Py_None; // immortal, no incref needed
115114
}
116115
break;
117116
case Py_T_OBJECT_EX:
117+
#ifdef Py_GIL_DISABLED
118+
v = _Py_XGetRef((PyObject **)addr);
119+
if (v == NULL) {
120+
PyErr_Format(PyExc_AttributeError,
121+
"'%T' object has no attribute '%s'",
122+
(PyObject *)obj_addr, l->name);
123+
}
124+
#else
118125
v = member_get_object(addr, obj_addr, l);
119-
#ifndef Py_GIL_DISABLED
120126
Py_XINCREF(v);
121-
#else
122-
if (v != NULL) {
123-
if (!_Py_TryIncrefCompare((PyObject **) addr, v)) {
124-
Py_BEGIN_CRITICAL_SECTION((PyObject *) obj_addr);
125-
v = member_get_object(addr, obj_addr, l);
126-
Py_XINCREF(v);
127-
Py_END_CRITICAL_SECTION();
128-
}
129-
}
130127
#endif
131128
break;
132129
case Py_T_LONGLONG:
@@ -320,11 +317,15 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
320317
break;
321318
}
322319
case _Py_T_OBJECT:
323-
case Py_T_OBJECT_EX:
324-
Py_BEGIN_CRITICAL_SECTION(obj);
320+
case Py_T_OBJECT_EX: {
321+
// Lock-free atomic exchange; matches _STORE_ATTR_SLOT.
322+
PyObject *new_v = Py_XNewRef(v);
323+
#ifdef Py_GIL_DISABLED
324+
oldv = (PyObject *)_Py_atomic_exchange_ptr((void **)addr, new_v);
325+
#else
325326
oldv = *(PyObject **)addr;
326-
FT_ATOMIC_STORE_PTR_RELEASE(*(PyObject **)addr, Py_XNewRef(v));
327-
Py_END_CRITICAL_SECTION();
327+
*(PyObject **)addr = new_v;
328+
#endif
328329
if (v == NULL && oldv == NULL && l->type == Py_T_OBJECT_EX) {
329330
// Raise an exception when attempting to delete an already deleted
330331
// attribute.
@@ -336,6 +337,7 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
336337
}
337338
Py_XDECREF(oldv);
338339
break;
340+
}
339341
case Py_T_CHAR: {
340342
const char *string;
341343
Py_ssize_t len;

Tools/cases_generator/analyzer.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,7 @@ def has_error_without_pop(op: parser.CodeDef) -> bool:
700700
"_Py_TryIncrefCompare",
701701
"_Py_TryIncrefCompareStackRef",
702702
"_Py_atomic_compare_exchange_uint8",
703+
"_Py_atomic_exchange_ptr",
703704
"_Py_atomic_load_ptr_acquire",
704705
"_Py_atomic_load_uintptr_relaxed",
705706
"_Py_set_eval_breaker_bit",

0 commit comments

Comments
 (0)