Skip to content

Commit 8a7f057

Browse files
[3.14] gh-146270: Fix PyMember_SetOne(..., NULL) not being atomic (gh-148800) (#149460)
Fixes a sequential consistency bug whereby two threads that are deleting a struct member may observe both their deletions to be successful. (cherry picked from commit 1bdfc0f) Co-authored-by: Daniele Parmeggiani <8658291+dpdani@users.noreply.github.com>
1 parent 5b33e9c commit 8a7f057

3 files changed

Lines changed: 41 additions & 21 deletions

File tree

Lib/test/test_free_threading/test_slots.py

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,19 @@ def run_in_threads(targets):
1616
thread.join()
1717

1818

19+
class Spam:
20+
__slots__ = [
21+
"eggs",
22+
]
23+
24+
def __init__(self, initial_value):
25+
self.eggs = initial_value
26+
27+
1928
@threading_helper.requires_working_threading()
2029
class TestSlots(TestCase):
2130

2231
def test_object(self):
23-
class Spam:
24-
__slots__ = [
25-
"eggs",
26-
]
27-
28-
def __init__(self, initial_value):
29-
self.eggs = initial_value
30-
3132
spam = Spam(0)
3233
iters = 20_000
3334

@@ -43,6 +44,24 @@ def reader():
4344

4445
run_in_threads([writer, reader, reader, reader])
4546

47+
def test_del_object_is_atomic(self):
48+
# Testing whether the implementation of `del slots_object.attribute`
49+
# removes the attribute atomically, thus avoiding non-sequentially-
50+
# consistent behaviors.
51+
# https://github.com/python/cpython/issues/146270
52+
def deleter(spam, successes):
53+
try:
54+
del spam.eggs
55+
successes.append(True)
56+
except AttributeError:
57+
successes.append(False)
58+
59+
for _ in range(10):
60+
spam = Spam(0)
61+
successes = []
62+
threading_helper.run_concurrently(deleter, nthreads=4, args=(spam, successes))
63+
self.assertEqual(sum(successes), 1)
64+
4665
def test_T_BOOL(self):
4766
spam_old = _testcapi._test_structmembersType_OldAPI()
4867
spam_new = _testcapi._test_structmembersType_NewAPI()
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix a sequential consistency bug in ``structmember.c``.

Python/structmember.c

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -161,19 +161,10 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
161161
PyErr_SetString(PyExc_AttributeError, "readonly attribute");
162162
return -1;
163163
}
164-
if (v == NULL) {
165-
if (l->type == Py_T_OBJECT_EX) {
166-
/* Check if the attribute is set. */
167-
if (*(PyObject **)addr == NULL) {
168-
PyErr_SetString(PyExc_AttributeError, l->name);
169-
return -1;
170-
}
171-
}
172-
else if (l->type != _Py_T_OBJECT) {
173-
PyErr_SetString(PyExc_TypeError,
174-
"can't delete numeric/char attribute");
175-
return -1;
176-
}
164+
if (v == NULL && l->type != Py_T_OBJECT_EX && l->type != _Py_T_OBJECT) {
165+
PyErr_SetString(PyExc_TypeError,
166+
"can't delete numeric/char attribute");
167+
return -1;
177168
}
178169
switch (l->type) {
179170
case Py_T_BOOL:{
@@ -324,6 +315,15 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
324315
oldv = *(PyObject **)addr;
325316
FT_ATOMIC_STORE_PTR_RELEASE(*(PyObject **)addr, Py_XNewRef(v));
326317
Py_END_CRITICAL_SECTION();
318+
if (v == NULL && oldv == NULL && l->type == Py_T_OBJECT_EX) {
319+
// Raise an exception when attempting to delete an already deleted
320+
// attribute.
321+
// Differently from Py_T_OBJECT_EX, _Py_T_OBJECT does not raise an
322+
// exception here (PyMember_GetOne will return Py_None instead of
323+
// NULL).
324+
PyErr_SetString(PyExc_AttributeError, l->name);
325+
return -1;
326+
}
327327
Py_XDECREF(oldv);
328328
break;
329329
case Py_T_CHAR: {

0 commit comments

Comments
 (0)