From 5820ba8c484d5c4f177bbc060a20b937993710f0 Mon Sep 17 00:00:00 2001 From: jg Date: Wed, 14 Oct 2026 23:00:15 +0100 Subject: [PATCH 01/10] Avoid re-locking both sets in set.issuperset() on the free-threaded build `set_issuperset_impl()` is already locked but calls `set_issubset` which just wraps a lock to `set_issubset_impl` (cherry picked from commit d4e412ba723df41344676f1c902172814141ab62) --- Objects/setobject.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 8fdd1eb26118c0a..7b31acf132a7639 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -2418,6 +2418,9 @@ set_issubset_impl(PySetObject *so, PyObject *other) Py_ssize_t pos = 0; int rv; + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(so); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(other); + if (!PyAnySet_Check(other)) { PyObject *tmp = set_intersection(so, other); if (tmp == NULL) { @@ -2460,7 +2463,7 @@ set_issuperset_impl(PySetObject *so, PyObject *other) /*[clinic end generated code: output=ecf00ce552c09461 input=5f2e1f262e6e4ccc]*/ { if (PyAnySet_Check(other)) { - return set_issubset(other, (PyObject *)so); + return set_issubset_impl((PySetObject *)other, (PyObject *)so); } PyObject *key, *it = PyObject_GetIter(other); From 0a4c7880e042c5bf065c6a958e4acaa7e3ae5e5c Mon Sep 17 00:00:00 2001 From: jg Date: Fri, 16 Oct 2026 17:48:05 +0100 Subject: [PATCH 02/10] Avoid re-locking the set in set.discard() and set.remove() on the free-threaded build set_lookkey() is shared by callers that run without the set's lock and by callers that already hold it, so it takes the critical section on a mutable set itself. set.discard() and set.remove() hold that lock, taken by their Argument Clinic wrappers, and reached set_lookkey() through set_discard_entry(), so every call re-entered the critical section and went through _PyCriticalSection_BeginSlow(). Split set_lookkey() into a lock-held core and a locking wrapper, and use the core from set_discard_entry(): all of its callers (set.discard, set.remove, PySet_Discard and the difference-update helpers) hold the lock, and the helpers already assert it. Measured on the free-threaded build: s.discard(1); s.add(1) 34 -> 23 ns. --- Objects/setobject.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 7b31acf132a7639..a594a1ac5c814c7 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -410,8 +410,11 @@ set_insert_clean(setentry *table, size_t mask, PyObject *key, Py_hash_t hash) /* ======== End logic for probing the hash table ========================== */ /* ======================================================================== */ +// Like set_lookkey(), but the caller holds the critical section on a mutable +// set (or has exclusive access to it), so don't lock it again. static int -set_lookkey(PySetObject *so, PyObject *key, Py_hash_t hash, setentry **epp) +set_lookkey_lock_held(PySetObject *so, PyObject *key, Py_hash_t hash, + setentry **epp) { int status; if (PyFrozenSet_CheckExact(so)) { @@ -419,12 +422,10 @@ set_lookkey(PySetObject *so, PyObject *key, Py_hash_t hash, setentry **epp) set_compare_frozenset); } else { - Py_BEGIN_CRITICAL_SECTION(so); do { status = set_do_lookup(so, so->table, so->mask, key, hash, epp, set_compare_entry_lock_held); } while (status == SET_LOOKKEY_CHANGED); - Py_END_CRITICAL_SECTION(); } assert(status == SET_LOOKKEY_FOUND || status == SET_LOOKKEY_NO_MATCH || @@ -432,6 +433,19 @@ set_lookkey(PySetObject *so, PyObject *key, Py_hash_t hash, setentry **epp) return status; } +static int +set_lookkey(PySetObject *so, PyObject *key, Py_hash_t hash, setentry **epp) +{ + if (PyFrozenSet_CheckExact(so)) { + return set_lookkey_lock_held(so, key, hash, epp); + } + int status; + Py_BEGIN_CRITICAL_SECTION(so); + status = set_lookkey_lock_held(so, key, hash, epp); + Py_END_CRITICAL_SECTION(); + return status; +} + #ifdef Py_GIL_DISABLED static int set_lookkey_threadsafe(PySetObject *so, PyObject *key, Py_hash_t hash) @@ -581,7 +595,7 @@ set_discard_entry(PySetObject *so, PyObject *key, Py_hash_t hash) { setentry *entry; PyObject *old_key; - int status = set_lookkey(so, key, hash, &entry); + int status = set_lookkey_lock_held(so, key, hash, &entry); if (status < 0) { return -1; } From 5ca398f22a1883ed1319222e7ca830d260c04ab6 Mon Sep 17 00:00:00 2001 From: jg Date: Fri, 16 Oct 2026 17:48:30 +0100 Subject: [PATCH 03/10] 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. --- 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 576731e24e95524d28680314503ff023a0a27c0c Mon Sep 17 00:00:00 2001 From: jg Date: Fri, 16 Oct 2026 17:48:32 +0100 Subject: [PATCH 04/10] 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. --- 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 ead09765894b752952472ead9e5b28155a72ae0e Mon Sep 17 00:00:00 2001 From: jg Date: Fri, 16 Oct 2026 17:48:33 +0100 Subject: [PATCH 05/10] 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. --- 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 dc0fe28617d5adf63fe63a05d4e14f9efddf0ed1 Mon Sep 17 00:00:00 2001 From: jg Date: Thu, 15 Oct 2026 21:44:28 +0100 Subject: [PATCH 06/10] News entry (cherry picked from commit 5867c21909b3181e9fd9c59f0a0ce4b564e7d470) --- .../2026-10-15-21-44-08.gh-issue-159486.2yJkXO.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-21-44-08.gh-issue-159486.2yJkXO.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-21-44-08.gh-issue-159486.2yJkXO.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-21-44-08.gh-issue-159486.2yJkXO.rst new file mode 100644 index 000000000000000..d493998dbef9dd9 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-21-44-08.gh-issue-159486.2yJkXO.rst @@ -0,0 +1 @@ +Speed up ``set >= set``, ``set > set`` and ``set.issuperset()`` on the free-threaded build. From 180a45552a5997b37e23b9ad866d059537ff6f70 Mon Sep 17 00:00:00 2001 From: jg Date: Fri, 16 Oct 2026 17:55:45 +0100 Subject: [PATCH 07/10] gh-159558: Add NEWS entries for the set and ctypes critical-section re-entry fixes --- .../2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst | 2 ++ .../Library/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst | 4 ++++ 2 files changed, 6 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst create mode 100644 Misc/NEWS.d/next/Library/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst new file mode 100644 index 000000000000000..80f3c5ed276bf15 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst @@ -0,0 +1,2 @@ +Speed up :meth:`set.discard` and :meth:`set.remove` on the free-threaded build +by not re-entering the critical section already held on the set. diff --git a/Misc/NEWS.d/next/Library/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst b/Misc/NEWS.d/next/Library/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst new file mode 100644 index 000000000000000..a8b8570365f3f48 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.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. From b5102fc39b3ce148ca672949c3971e16e83b8950 Mon Sep 17 00:00:00 2001 From: jg Date: Fri, 16 Oct 2026 18:22:29 +0100 Subject: [PATCH 08/10] Move the ctypes changes to a separate PR Revert the three ctypes commits and drop their NEWS entry so that this PR only changes set; the ctypes part is resubmitted on its own. --- ...-10-16-17-55-45.gh-issue-159558.ftCsRe.rst | 4 - Modules/_ctypes/_ctypes.c | 74 ++++++------------- 2 files changed, 22 insertions(+), 56 deletions(-) delete mode 100644 Misc/NEWS.d/next/Library/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst diff --git a/Misc/NEWS.d/next/Library/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst b/Misc/NEWS.d/next/Library/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst deleted file mode 100644 index a8b8570365f3f48..000000000000000 --- a/Misc/NEWS.d/next/Library/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst +++ /dev/null @@ -1,4 +0,0 @@ -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 61a8452fd225f80..f0945e48aa3bf03 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -2865,33 +2865,28 @@ 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; + CDataObject *res = self; Py_BEGIN_CRITICAL_SECTION(self); - res = PyCData_GetContainer_lock_held(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:; Py_END_CRITICAL_SECTION(); return res; } @@ -2906,22 +2901,6 @@ 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) { @@ -2958,15 +2937,7 @@ KeepRef_lock_held(CDataObject *target, Py_ssize_t index, PyObject *keep) Py_DECREF(Py_None); return 0; } - 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); - } + ob = PyCData_GetContainer(target); if (ob == NULL) { Py_DECREF(keep); return -1; @@ -3563,7 +3534,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_lock_held(mem, index, result); + return KeepRef(mem, index, result); } @@ -5431,9 +5402,8 @@ _ctypes_Simple_value_set_impl(CDataObject *self, PyObject *value) if (!result) return -1; - /* 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); + /* consumes the refcount the setfunc returns */ + return KeepRef(self, 0, result); } @@ -5732,15 +5702,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_lock_held(self, 1, value)) + if (-1 == KeepRef(self, 1, value)) return -1; - keep = GetKeepedObjects_lock_held(dst); + keep = GetKeepedObjects(dst); if (keep == NULL) return -1; Py_INCREF(keep); - return KeepRef_lock_held(self, 0, keep); + return KeepRef(self, 0, keep); } static int From f13c0ee2b5400467ebd59ef878810f8631d979c7 Mon Sep 17 00:00:00 2001 From: jg Date: Fri, 16 Oct 2026 18:24:05 +0100 Subject: [PATCH 09/10] gh-159558: Combine the NEWS entries --- .../2026-10-15-21-44-08.gh-issue-159486.2yJkXO.rst | 1 - .../2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) delete mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-21-44-08.gh-issue-159486.2yJkXO.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-21-44-08.gh-issue-159486.2yJkXO.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-21-44-08.gh-issue-159486.2yJkXO.rst deleted file mode 100644 index d493998dbef9dd9..000000000000000 --- a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-21-44-08.gh-issue-159486.2yJkXO.rst +++ /dev/null @@ -1 +0,0 @@ -Speed up ``set >= set``, ``set > set`` and ``set.issuperset()`` on the free-threaded build. diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst index 80f3c5ed276bf15..db469b159e023f7 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-16-17-55-45.gh-issue-159558.ftCsRe.rst @@ -1,2 +1,3 @@ -Speed up :meth:`set.discard` and :meth:`set.remove` on the free-threaded build -by not re-entering the critical section already held on the set. +Speed up ``set >= set``, ``set > set``, :meth:`set.issuperset`, :meth:`set.discard` +and :meth:`set.remove` on the free-threaded build by not re-entering a critical +section already held on the set. From f9b171821b0cc6fad6d7f1dc4b34e2923f146336 Mon Sep 17 00:00:00 2001 From: jg Date: Fri, 16 Oct 2026 18:32:48 +0100 Subject: [PATCH 10/10] gh-159558: Drop a redundant comment --- Objects/setobject.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index a594a1ac5c814c7..e279a90d4483dc7 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -410,8 +410,6 @@ set_insert_clean(setentry *table, size_t mask, PyObject *key, Py_hash_t hash) /* ======== End logic for probing the hash table ========================== */ /* ======================================================================== */ -// Like set_lookkey(), but the caller holds the critical section on a mutable -// set (or has exclusive access to it), so don't lock it again. static int set_lookkey_lock_held(PySetObject *so, PyObject *key, Py_hash_t hash, setentry **epp)