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. diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index f0945e48aa3bf03..61a8452fd225f80 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; } @@ -2901,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) { @@ -2937,7 +2958,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; @@ -3534,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); } @@ -5402,8 +5431,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); } @@ -5702,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