Fix race condition with py::make_key_iterator in free threading (#5971)
* Fix race condition with py::make_key_iterator in free threading
The creation of the iterator class needs to be synchronized.
* style: pre-commit fixes
* Use PyCriticalSection_BeginMutex instead of recursive mutex
* style: pre-commit fixes
* Make pycritical_section non-copyable and non-movable
The pycritical_section class is a RAII wrapper that manages a Python
critical section lifecycle:
- Acquires the critical section in the constructor via
PyCriticalSection_BeginMutex
- Releases it in the destructor via PyCriticalSection_End
- Holds a reference to a pymutex
Allowing copy or move operations would be dangerous:
1. Copy: Both the original and copied objects would call
PyCriticalSection_End on the same PyCriticalSection object in their
destructors, leading to double-unlock and undefined behavior.
2. Move: The moved-from object's destructor would still run and attempt
to end the critical section, while the moved-to object would also try
to end it, again causing double-unlock.
This follows the same pattern used by other RAII lock guards in the
codebase, such as gil_scoped_acquire and gil_scoped_release, which also
explicitly delete copy/move operations to prevent similar issues.
By explicitly deleting these operations, we prevent accidental misuse
and ensure the critical section is properly managed by a single RAII
object throughout its lifetime.
* Drop Python 3.13t support from CI
Python 3.13t was experimental, while Python 3.14t is not. This PR
uses PyCriticalSection_BeginMutex which is only available in Python
3.14+, making Python 3.13t incompatible with the changes.
Removed all Python 3.13t CI jobs:
- ubuntu-latest, 3.13t (standard-large matrix)
- macos-15-intel, 3.13t (standard-large matrix)
- windows-latest, 3.13t (standard-large matrix)
- manylinux job testing 3.13t
This aligns with the decision to drop Python 3.13t support as
discussed in PR #5971.
* Add Python 3.13 (default) replacement jobs for removed 3.13t jobs
After removing Python 3.13t support (incompatible with PyCriticalSection_BeginMutex
which requires Python 3.14+), we're adding replacement jobs using Python 3.13
(default) to maintain test coverage in key dimensions:
1. ubuntu-latest, Python 3.13: C++20 + DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION
- Replaces: ubuntu-latest, 3.13t with same config
- Maintains coverage for this specific configuration combination
2. macos-15-intel, Python 3.13: C++11
- Replaces: macos-15-intel, 3.13t with same config
- Maintains macOS coverage for Python 3.13
3. manylinux (musllinux), Python 3.13: GIL testing
- Replaces: manylinux, 3.13t job
- Maintains manylinux/musllinux container testing coverage
These additions are proposed to get feedback on which jobs should be kept
to maintain appropriate test coverage without the experimental 3.13t builds.
* ci: run in free-threading mode a bit more on 3.14
* Revert "ci: run in free-threading mode a bit more on 3.14"
This reverts commit 91189c9242e787922d26ca467710dcc494871b82.
Reason: https://github.com/pybind/pybind11/pull/5971#issuecomment-3831321903
* Reapply "ci: run in free-threading mode a bit more on 3.14"
This reverts commit f3197de97557a86a9a73df7890be1c227a7c4c59.
After #5972 is/was merged, tests should pass (already tested under #5980).
See also https://github.com/pybind/pybind11/pull/5972#discussion_r2752674989
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgkio@gmail.com>
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index cda2c21..a0965fa 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -84,7 +84,7 @@
python-version: '3.12'
cmake-args: -DPYBIND11_TEST_SMART_HOLDER=ON -DPYBIND11_SIMPLE_GIL_MANAGEMENT=ON
- runs-on: ubuntu-latest
- python-version: '3.13t'
+ python-version: '3.14t'
cmake-args: -DCMAKE_CXX_STANDARD=20 -DPYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION=ON
- runs-on: ubuntu-latest
python-version: '3.14'
@@ -102,12 +102,12 @@
- runs-on: macos-15-intel
python-version: '3.11'
cmake-args: -DPYBIND11_TEST_SMART_HOLDER=ON
+ - runs-on: macos-15-intel
+ python-version: '3.13'
+ cmake-args: -DCMAKE_CXX_STANDARD=11
- runs-on: macos-latest
python-version: '3.12'
cmake-args: -DCMAKE_CXX_STANDARD=17 -DPYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION=ON
- - runs-on: macos-15-intel
- python-version: '3.13t'
- cmake-args: -DCMAKE_CXX_STANDARD=11
- runs-on: macos-latest
python-version: '3.14t'
cmake-args: -DCMAKE_CXX_STANDARD=20
@@ -139,9 +139,6 @@
python-version: '3.13'
cmake-args: -DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreadedDebugDLL
- runs-on: windows-latest
- python-version: '3.13t'
- cmake-args: -DCMAKE_CXX_STANDARD=17
- - runs-on: windows-latest
python-version: '3.14'
cmake-args: -DCMAKE_CXX_STANDARD=20
- runs-on: windows-latest
@@ -240,7 +237,7 @@
manylinux:
- name: Manylinux on 🐍 3.13t • GIL
+ name: Manylinux on 🐍 3.14t
if: github.event.pull_request.draft == false
runs-on: ubuntu-latest
timeout-minutes: 40
@@ -257,7 +254,7 @@
run: uv tool install ninja
- name: Configure via preset
- run: cmake --preset venv -DPYBIND11_CREATE_WITH_UV=python3.13t
+ run: cmake --preset venv -DPYBIND11_CREATE_WITH_UV=python3.14t
- name: Build C++11
run: cmake --build --preset venv
diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h
index 7cfa5da..b9b0f08 100644
--- a/include/pybind11/detail/internals.h
+++ b/include/pybind11/detail/internals.h
@@ -230,6 +230,7 @@
#ifdef Py_GIL_DISABLED
// Wrapper around PyMutex to provide BasicLockable semantics
class pymutex {
+ friend class pycritical_section;
PyMutex mutex;
public:
@@ -238,6 +239,23 @@
void unlock() { PyMutex_Unlock(&mutex); }
};
+class pycritical_section {
+ pymutex &mutex;
+ PyCriticalSection cs;
+
+public:
+ explicit pycritical_section(pymutex &m) : mutex(m) {
+ PyCriticalSection_BeginMutex(&cs, &mutex.mutex);
+ }
+ ~pycritical_section() { PyCriticalSection_End(&cs); }
+
+ // Non-copyable and non-movable to prevent double-unlock
+ pycritical_section(const pycritical_section &) = delete;
+ pycritical_section &operator=(const pycritical_section &) = delete;
+ pycritical_section(pycritical_section &&) = delete;
+ pycritical_section &operator=(pycritical_section &&) = delete;
+};
+
// Instance map shards are used to reduce mutex contention in free-threaded Python.
struct instance_map_shard {
instance_map registered_instances;
@@ -905,7 +923,7 @@
}
#ifdef Py_GIL_DISABLED
-# define PYBIND11_LOCK_INTERNALS(internals) std::unique_lock<pymutex> lock((internals).mutex)
+# define PYBIND11_LOCK_INTERNALS(internals) pycritical_section lock((internals).mutex)
#else
# define PYBIND11_LOCK_INTERNALS(internals)
#endif
@@ -934,7 +952,7 @@
get_local_internals().registered_exception_translators)) {
auto &internals = get_internals();
#ifdef Py_GIL_DISABLED
- std::unique_lock<pymutex> lock((internals).exception_translator_mutex);
+ pycritical_section lock((internals).exception_translator_mutex);
#endif
auto &local_internals = get_local_internals();
return cb(internals.registered_exception_translators,
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
index 02d2e72..f88fc20 100644
--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -3173,6 +3173,7 @@
using state = detail::iterator_state<Access, Policy, Iterator, Sentinel, ValueType, Extra...>;
// TODO: state captures only the types of Extra, not the values
+ PYBIND11_LOCK_INTERNALS(get_internals());
if (!detail::get_type_info(typeid(state), false)) {
class_<state>(handle(), "iterator", pybind11::module_local())
.def(
