]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-146270: Fix `PyMember_SetOne(..., NULL)` not being atomic (gh-148800)
authorDaniele Parmeggiani <8658291+dpdani@users.noreply.github.com>
Wed, 6 May 2026 13:50:24 +0000 (14:50 +0100)
committerGitHub <noreply@github.com>
Wed, 6 May 2026 13:50:24 +0000 (09:50 -0400)
Fixes a sequential consistency bug whereby two threads that are deleting a struct member may observe both their deletions to be successful.

Lib/test/test_free_threading/test_slots.py
Misc/NEWS.d/next/Core_and_Builtins/2026-04-20-15-25-55.gh-issue-146270.qZYfyc.rst [new file with mode: 0644]
Python/structmember.c

index a3b9f4b0175ae7693e2c60f8b351623d742bd30a..a73525e1bebfb49d74ff95432a741abd7bc2bbbf 100644 (file)
@@ -16,18 +16,19 @@ def run_in_threads(targets):
         thread.join()
 
 
+class Spam:
+    __slots__ = [
+        "eggs",
+    ]
+
+    def __init__(self, initial_value):
+        self.eggs = initial_value
+
+
 @threading_helper.requires_working_threading()
 class TestSlots(TestCase):
 
     def test_object(self):
-        class Spam:
-            __slots__ = [
-                "eggs",
-            ]
-
-            def __init__(self, initial_value):
-                self.eggs = initial_value
-
         spam = Spam(0)
         iters = 20_000
 
@@ -43,6 +44,24 @@ class TestSlots(TestCase):
 
         run_in_threads([writer, reader, reader, reader])
 
+    def test_del_object_is_atomic(self):
+        # Testing whether the implementation of `del slots_object.attribute`
+        # removes the attribute atomically, thus avoiding non-sequentially-
+        # consistent behaviors.
+        # https://github.com/python/cpython/issues/146270
+        def deleter(spam, successes):
+            try:
+                del spam.eggs
+                successes.append(True)
+            except AttributeError:
+                successes.append(False)
+
+        for _ in range(10):
+            spam = Spam(0)
+            successes = []
+            threading_helper.run_concurrently(deleter, nthreads=4, args=(spam, successes))
+            self.assertEqual(sum(successes), 1)
+
     def test_T_BOOL(self):
         spam_old = _testcapi._test_structmembersType_OldAPI()
         spam_new = _testcapi._test_structmembersType_NewAPI()
diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-04-20-15-25-55.gh-issue-146270.qZYfyc.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-04-20-15-25-55.gh-issue-146270.qZYfyc.rst
new file mode 100644 (file)
index 0000000..46c292e
--- /dev/null
@@ -0,0 +1 @@
+Fix a sequential consistency bug in ``structmember.c``.
index b88e13ac0462b80c27fc2f2d2ba8c36bb3709fad..adea8216b8796b5fb1cc293f0b99916a8904ccf7 100644 (file)
@@ -171,19 +171,10 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
         PyErr_SetString(PyExc_AttributeError, "readonly attribute");
         return -1;
     }
-    if (v == NULL) {
-        if (l->type == Py_T_OBJECT_EX) {
-            /* Check if the attribute is set. */
-            if (*(PyObject **)addr == NULL) {
-                PyErr_SetString(PyExc_AttributeError, l->name);
-                return -1;
-            }
-        }
-        else if (l->type != _Py_T_OBJECT) {
-            PyErr_SetString(PyExc_TypeError,
-                            "can't delete numeric/char attribute");
-            return -1;
-        }
+    if (v == NULL && l->type != Py_T_OBJECT_EX && l->type != _Py_T_OBJECT) {
+        PyErr_SetString(PyExc_TypeError,
+                        "can't delete numeric/char attribute");
+        return -1;
     }
     switch (l->type) {
     case Py_T_BOOL:{
@@ -334,6 +325,15 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
         oldv = *(PyObject **)addr;
         FT_ATOMIC_STORE_PTR_RELEASE(*(PyObject **)addr, Py_XNewRef(v));
         Py_END_CRITICAL_SECTION();
+        if (v == NULL && oldv == NULL && l->type == Py_T_OBJECT_EX) {
+            // Raise an exception when attempting to delete an already deleted
+            // attribute.
+            // Differently from Py_T_OBJECT_EX, _Py_T_OBJECT does not raise an
+            // exception here (PyMember_GetOne will return Py_None instead of
+            // NULL).
+            PyErr_SetString(PyExc_AttributeError, l->name);
+            return -1;
+        }
         Py_XDECREF(oldv);
         break;
     case Py_T_CHAR: {