gh-159558: Avoid re-entering an already held critical section in ctypes setters (GH-159562)

Author: johng

Misc/NEWS.d/next/Library/2026-10-16-18-23-03.gh-issue-159558.ctRelk.rst

@@ -0,0 +1,4 @@
+Speed up assignment to the ``value`` attribute of simple :mod:`ctypes` types, to
+the ``contents`` attribute of :mod:`ctypes` pointers and to :mod:`ctypes`
+structure fields, array items and pointer items on the free-threaded build by
+not re-entering the critical section already held on the object.

Modules/_ctypes/_ctypes.c

@@ -2888,28 +2888,33 @@ static PyType_Spec pycfuncptr_type_spec = {
  * Code to keep needed objects alive
  */
 
+static CDataObject *
+PyCData_GetContainer_lock_held(CDataObject *root)
+{
+    assert(root->b_base == NULL);
+    _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(root);
+    if (root->b_objects == NULL) {
+        if (root->b_length) {
+            root->b_objects = PyDict_New();
+            if (root->b_objects == NULL) {
+                return NULL;
+            }
+        } else {
+            root->b_objects = Py_NewRef(Py_None);
+        }
+    }
+    return root;
+}
+
 static CDataObject *
 PyCData_GetContainer(CDataObject *self)
 {
     while (self->b_base) {
         self = self->b_base;
     }
-    CDataObject *res = self;
+    CDataObject *res;
     Py_BEGIN_CRITICAL_SECTION(self);
-    // avoid using return directly in this block because critical section
-    // needs to be released before returning
-    if (self->b_objects == NULL) {
-        if (self->b_length) {
-            self->b_objects = PyDict_New();
-            if (self->b_objects == NULL) {
-                res = NULL;
-                goto exit;
-            }
-        } else {
-            self->b_objects = Py_NewRef(Py_None);
-        }
-    }
-exit:;
+    res = PyCData_GetContainer_lock_held(self);
     Py_END_CRITICAL_SECTION();
     return res;
 }
@@ -2924,6 +2929,22 @@ GetKeepedObjects(CDataObject *target)
     return container->b_objects;
 }
 
+/* Like GetKeepedObjects(), for a caller that already holds target's lock. */
+static PyObject *
+GetKeepedObjects_lock_held(CDataObject *target)
+{
+    CDataObject *container;
+    if (target->b_base == NULL) {
+        container = PyCData_GetContainer_lock_held(target);
+    }
+    else {
+        container = PyCData_GetContainer(target);
+    }
+    if (container == NULL)
+        return NULL;
+    return container->b_objects;
+}
+
 static PyObject *
 unique_key(CDataObject *target, Py_ssize_t index)
 {
@@ -2960,7 +2981,15 @@ KeepRef_lock_held(CDataObject *target, Py_ssize_t index, PyObject *keep)
         Py_DECREF(Py_None);
         return 0;
     }
-    ob = PyCData_GetContainer(target);
+    if (target->b_base == NULL) {
+        // target is its own root and the caller already holds its lock:
+        // re-entering the critical section would be a nested same-object
+        // lock (failed CAS + slow path) on every field assignment.
+        ob = PyCData_GetContainer_lock_held(target);
+    }
+    else {
+        ob = PyCData_GetContainer(target);
+    }
     if (ob == NULL) {
         Py_DECREF(keep);
         return -1;
@@ -3564,7 +3593,7 @@ PyCData_set(ctypes_state *st,
     /* KeepRef steals a refcount from it's last argument */
     /* If KeepRef fails, we are stumped.  The dst memory block has already
        been changed */
-    return KeepRef(mem, index, result);
+    return KeepRef_lock_held(mem, index, result);
 }
 
 
@@ -5452,8 +5481,9 @@ _ctypes_Simple_value_set_impl(CDataObject *self, PyObject *value)
     if (!result)
         return -1;
 
-    /* consumes the refcount the setfunc returns */
-    return KeepRef(self, 0, result);
+    /* consumes the refcount the setfunc returns; self is already locked
+       by the critical section taken by the Argument Clinic wrapper */
+    return KeepRef_lock_held(self, 0, result);
 }
 
 
@@ -5763,15 +5793,15 @@ Pointer_set_contents_lock_held(PyObject *op, PyObject *value, void *closure)
        'value' itself as the second item of the b_objects list, additionally.
     */
     Py_INCREF(value);
-    if (-1 == KeepRef(self, 1, value))
+    if (-1 == KeepRef_lock_held(self, 1, value))
         return -1;
 
-    keep = GetKeepedObjects(dst);
+    keep = GetKeepedObjects_lock_held(dst);
     if (keep == NULL)
         return -1;
 
     Py_INCREF(keep);
-    return KeepRef(self, 0, keep);
+    return KeepRef_lock_held(self, 0, keep);
 }
 
 static int