From 1a8ecd751f9cf76a61f92b6c44b878ae2f2c576c Mon Sep 17 00:00:00 2001 From: jg Date: Fri, 16 Oct 2026 17:48:30 +0100 Subject: [PATCH 1/4] ctypes: do not re-lock the object in the Simple.value setter on the free-threaded build KeepRef() takes the critical section on its target so that it is safe from any caller, and for a type that keeps a reference to the stored object it then calls PyCData_GetContainer(), which takes the critical section on the root object. The Simple.value setter already holds the lock on self, taken by its Argument Clinic wrapper, and for a top-level object the root is self as well, so `c_char_p().value = b"x"` entered the same critical section three times and `c_int().value = 1` twice (the "nothing to keep" early return sits inside the locked region). Add PyCData_GetContainer_lock_held() for a caller that holds the root's lock, use it from KeepRef_lock_held() when the target is its own root, and call KeepRef_lock_held() from the setter. Measured on the free-threaded build: c_int().value = 1 35 -> 22 ns, c_char_p().value = b"x" 39 -> 22 ns. (cherry picked from commit 5ca398f22a1883ed1319222e7ca830d260c04ab6) --- Modules/_ctypes/_ctypes.c | 50 +++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index f0945e48aa3bf03..be15eb2dac41790 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -2865,28 +2865,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; } @@ -2937,7 +2942,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; @@ -5402,8 +5415,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); } From 60f8473861805750dd91a88859950f1063a8f957 Mon Sep 17 00:00:00 2001 From: jg Date: Fri, 16 Oct 2026 17:48:32 +0100 Subject: [PATCH 2/4] ctypes: do not re-lock the pointer or its target in the Pointer.contents setter The setter runs under a two-object critical section on the pointer and the new target, then calls KeepRef() twice on the pointer and GetKeepedObjects() on the target; both helpers lock their argument again. Call the lock-held variants instead, adding GetKeepedObjects_lock_held() for the target. Measured on the free-threaded build: p.contents = c_int(2) 86 -> 58 ns. (cherry picked from commit 576731e24e95524d28680314503ff023a0a27c0c) --- Modules/_ctypes/_ctypes.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index be15eb2dac41790..e27804077f58f01 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -2906,6 +2906,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) { @@ -5716,15 +5732,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 From 0fe7b59fe81d72fa60b5402523d4bfec02aa872a Mon Sep 17 00:00:00 2001 From: jg Date: Fri, 16 Oct 2026 17:48:33 +0100 Subject: [PATCH 3/4] ctypes: do not re-lock the destination in PyCData_set() PyCData_set() already requires its caller to hold the critical section on the destination object and asserts it, but finished by calling KeepRef(), which takes that lock again. Every store of a Python object into a Structure or Union field, an Array item or a Pointer item went through it. Call KeepRef_lock_held() instead. Measured on the free-threaded build: Structure int field assignment 38 -> 24 ns, c_char_p field assignment 69 -> 54 ns. (cherry picked from commit ead09765894b752952472ead9e5b28155a72ae0e) --- Modules/_ctypes/_ctypes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index e27804077f58f01..61a8452fd225f80 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -3563,7 +3563,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); } From 0529fb54aa3036d3c19ddef2c68b22dc9e720f5f Mon Sep 17 00:00:00 2001 From: jg Date: Fri, 16 Oct 2026 18:23:03 +0100 Subject: [PATCH 4/4] gh-159558: Add NEWS entry for the ctypes critical-section re-entry fixes --- .../Library/2026-10-16-18-23-03.gh-issue-159558.ctRelk.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2026-10-16-18-23-03.gh-issue-159558.ctRelk.rst diff --git a/Misc/NEWS.d/next/Library/2026-10-16-18-23-03.gh-issue-159558.ctRelk.rst b/Misc/NEWS.d/next/Library/2026-10-16-18-23-03.gh-issue-159558.ctRelk.rst new file mode 100644 index 000000000000000..a8b8570365f3f48 --- /dev/null +++ b/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.