From 3d20d9d76f427b5fa02281bc25272c7512b7fd40 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Fri, 30 May 2025 17:41:13 +0800
Subject: [PATCH 01/19] chore: handle null for `py::scoped_critical_section`

---
 include/pybind11/critical_section.h | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/include/pybind11/critical_section.h b/include/pybind11/critical_section.h
index e94ca765cb..cbff504692 100644
--- a/include/pybind11/critical_section.h
+++ b/include/pybind11/critical_section.h
@@ -13,18 +13,27 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 class scoped_critical_section {
 public:
 #ifdef Py_GIL_DISABLED
-    explicit scoped_critical_section(handle obj) : has2(false) {
-        PyCriticalSection_Begin(&section, obj.ptr());
+    scoped_critical_section(handle obj1, handle obj2) : m_ptr1(obj1.ptr()), m_ptr2(obj2.ptr()) {
+        if (m_ptr1 == nullptr) {
+            std::swap(m_ptr1, m_ptr2);
+        }
+        if (m_ptr2 != nullptr) {
+            PyCriticalSection2_Begin(&section2, m_ptr1, m_ptr2);
+        } else if (m_ptr1 != nullptr) {
+            PyCriticalSection_Begin(&section, m_ptr1);
+        }
     }
 
-    scoped_critical_section(handle obj1, handle obj2) : has2(true) {
-        PyCriticalSection2_Begin(&section2, obj1.ptr(), obj2.ptr());
+    explicit scoped_critical_section(handle obj) : m_ptr1(obj.ptr()) {
+        if (m_ptr1 != nullptr) {
+            PyCriticalSection_Begin(&section, m_ptr1);
+        }
     }
 
     ~scoped_critical_section() {
-        if (has2) {
+        if (m_ptr2 != nullptr) {
             PyCriticalSection2_End(&section2);
-        } else {
+        } else if (m_ptr1 != nullptr) {
             PyCriticalSection_End(&section);
         }
     }
@@ -39,7 +48,8 @@ class scoped_critical_section {
 
 private:
 #ifdef Py_GIL_DISABLED
-    bool has2;
+    PyObject *m_ptr1{nullptr};
+    PyObject *m_ptr2{nullptr};
     union {
         PyCriticalSection section;
         PyCriticalSection2 section2;

From 7f0b6ff88bf811216c41f3063714594e5f15c76a Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Sun, 1 Jun 2025 00:53:31 +0800
Subject: [PATCH 02/19] test: add tests for `py::scoped_critical_section`

---
 tests/CMakeLists.txt                   |   1 +
 tests/test_scoped_critical_section.cpp | 132 +++++++++++++++++++++++++
 tests/test_scoped_critical_section.py  |  23 +++++
 3 files changed, 156 insertions(+)
 create mode 100644 tests/test_scoped_critical_section.cpp
 create mode 100644 tests/test_scoped_critical_section.py

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 0e76e68786..2cf18c3547 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -166,6 +166,7 @@ set(PYBIND11_TEST_FILES
     test_potentially_slicing_weak_ptr
     test_python_multiple_inheritance
     test_pytypes
+    test_scoped_critical_section
     test_sequences_and_iterators
     test_smart_ptr
     test_stl
diff --git a/tests/test_scoped_critical_section.cpp b/tests/test_scoped_critical_section.cpp
new file mode 100644
index 0000000000..6ccfccdbea
--- /dev/null
+++ b/tests/test_scoped_critical_section.cpp
@@ -0,0 +1,132 @@
+#include <pybind11/critical_section.h>
+
+#include "catch.hpp"
+#include "pybind11_tests.h"
+
+#include <atomic>
+#include <thread>
+
+#ifdef PYBIND11_CPP20
+#    include <barrier>
+
+// Referenced test implementation: https://github.com/PyO3/pyo3/blob/v0.25.0/src/sync.rs#L874
+class BoolWrapper {
+public:
+    explicit BoolWrapper(bool value) : value_{value} {}
+    bool get() const { return value_.load(std::memory_order_acquire); }
+    void set(bool value) { value_.store(value, std::memory_order_release); }
+
+private:
+    std::atomic<bool> value_;
+};
+
+void test_scoped_critical_section(py::class_<BoolWrapper> &cls) {
+    auto barrier = std::barrier(2);
+    auto bool_wrapper = cls(false);
+
+    std::thread t1([&]() {
+        py::scoped_critical_section lock{bool_wrapper};
+        barrier.arrive_and_wait();
+        auto bw = bool_wrapper.cast<std::shared_ptr<BoolWrapper>>();
+        std::this_thread::sleep_for(std::chrono::milliseconds(10));
+        bw->set(true);
+    });
+
+    std::thread t2([&]() {
+        barrier.arrive_and_wait();
+        py::scoped_critical_section lock{bool_wrapper};
+        auto bw = bool_wrapper.cast<std::shared_ptr<BoolWrapper>>();
+        REQUIRE(bw->get() == true);
+    });
+
+    t1.join();
+    t2.join();
+}
+
+void test_scoped_critical_section2(py::class_<BoolWrapper> &cls) {
+    auto barrier = std::barrier(3);
+    auto bool_wrapper1 = cls(false);
+    auto bool_wrapper2 = cls(false);
+
+    std::thread t1([&]() {
+        py::scoped_critical_section lock{bool_wrapper1, bool_wrapper2};
+        barrier.arrive_and_wait();
+        std::this_thread::sleep_for(std::chrono::milliseconds(10));
+        auto bw1 = bool_wrapper1.cast<std::shared_ptr<BoolWrapper>>();
+        auto bw2 = bool_wrapper2.cast<std::shared_ptr<BoolWrapper>>();
+        bw1->set(true);
+        bw2->set(true);
+    });
+
+    std::thread t2([&]() {
+        barrier.arrive_and_wait();
+        py::scoped_critical_section lock{bool_wrapper1};
+        auto bw1 = bool_wrapper1.cast<std::shared_ptr<BoolWrapper>>();
+        REQUIRE(bw1->get() == true);
+    });
+
+    std::thread t3([&]() {
+        barrier.arrive_and_wait();
+        py::scoped_critical_section lock{bool_wrapper2};
+        auto bw2 = bool_wrapper2.cast<std::shared_ptr<BoolWrapper>>();
+        REQUIRE(bw2->get() == true);
+    });
+
+    t1.join();
+    t2.join();
+    t3.join();
+}
+
+void test_scoped_critical_section2_same_object_no_deadlock(py::class_<BoolWrapper> &cls) {
+    auto barrier = std::barrier(2);
+    auto bool_wrapper = cls(false);
+
+    std::thread t1([&]() {
+        py::scoped_critical_section lock{bool_wrapper, bool_wrapper};
+        barrier.arrive_and_wait();
+        std::this_thread::sleep_for(std::chrono::milliseconds(10));
+        auto bw = bool_wrapper.cast<std::shared_ptr<BoolWrapper>>();
+        bw->set(true);
+    });
+
+    std::thread t2([&]() {
+        barrier.arrive_and_wait();
+        py::scoped_critical_section lock{bool_wrapper};
+        auto bw = bool_wrapper.cast<std::shared_ptr<BoolWrapper>>();
+        REQUIRE(bw->get() == true);
+    });
+
+    t1.join();
+    t2.join();
+}
+#endif
+
+TEST_SUBMODULE(scoped_critical_section, m) {
+    m.attr("defined_THREAD_SANITIZER") =
+#if defined(THREAD_SANITIZER)
+        true;
+#else
+        false;
+#endif
+    m.attr("has_barrier") =
+#if defined(PYBIND11_CPP20)
+        true;
+#else
+        false;
+#endif
+
+#ifdef PYBIND11_CPP20
+    auto BoolWrapperClass = py::class_<BoolWrapper>(m, "BoolWrapper")
+                                .def(py::init<bool>())
+                                .def("get", &BoolWrapper::get)
+                                .def("set", &BoolWrapper::set);
+
+    m.def("test_scoped_critical_section",
+          [&]() -> void { test_scoped_critical_section(BoolWrapperClass); });
+    m.def("test_scoped_critical_section2",
+          [&]() -> void { test_scoped_critical_section2(BoolWrapperClass); });
+    m.def("test_scoped_critical_section2_same_object_no_deadlock", [&]() -> void {
+        test_scoped_critical_section2_same_object_no_deadlock(BoolWrapperClass);
+    });
+#endif
+}
diff --git a/tests/test_scoped_critical_section.py b/tests/test_scoped_critical_section.py
new file mode 100644
index 0000000000..49495fc1da
--- /dev/null
+++ b/tests/test_scoped_critical_section.py
@@ -0,0 +1,23 @@
+from __future__ import annotations
+
+import pytest
+
+from pybind11_tests import scoped_critical_section as m
+
+
+@pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
+def test_scoped_critical_section() -> None:
+    for _ in range(64):
+        m.test_scoped_critical_section()
+
+
+@pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
+def test_scoped_critical_section2() -> None:
+    for _ in range(64):
+        assert m.test_scoped_critical_section2()
+
+
+@pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
+def test_scoped_critical_section2_same_object_no_deadlock() -> None:
+    for _ in range(64):
+        m.test_scoped_critical_section2_same_object_no_deadlock()

From f7006c469b2a2b766527cfd52c498596103cfcf4 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Sun, 1 Jun 2025 02:03:35 +0800
Subject: [PATCH 03/19] test: use assert instead of REQUIRE

---
 tests/test_scoped_critical_section.cpp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/test_scoped_critical_section.cpp b/tests/test_scoped_critical_section.cpp
index 6ccfccdbea..efc207693d 100644
--- a/tests/test_scoped_critical_section.cpp
+++ b/tests/test_scoped_critical_section.cpp
@@ -1,9 +1,9 @@
 #include <pybind11/critical_section.h>
 
-#include "catch.hpp"
 #include "pybind11_tests.h"
 
 #include <atomic>
+#include <cassert>
 #include <thread>
 
 #ifdef PYBIND11_CPP20
@@ -36,7 +36,7 @@ void test_scoped_critical_section(py::class_<BoolWrapper> &cls) {
         barrier.arrive_and_wait();
         py::scoped_critical_section lock{bool_wrapper};
         auto bw = bool_wrapper.cast<std::shared_ptr<BoolWrapper>>();
-        REQUIRE(bw->get() == true);
+        assert(bw->get() == true);
     });
 
     t1.join();
@@ -62,14 +62,14 @@ void test_scoped_critical_section2(py::class_<BoolWrapper> &cls) {
         barrier.arrive_and_wait();
         py::scoped_critical_section lock{bool_wrapper1};
         auto bw1 = bool_wrapper1.cast<std::shared_ptr<BoolWrapper>>();
-        REQUIRE(bw1->get() == true);
+        assert(bw1->get() == true);
     });
 
     std::thread t3([&]() {
         barrier.arrive_and_wait();
         py::scoped_critical_section lock{bool_wrapper2};
         auto bw2 = bool_wrapper2.cast<std::shared_ptr<BoolWrapper>>();
-        REQUIRE(bw2->get() == true);
+        assert(bw2->get() == true);
     });
 
     t1.join();
@@ -93,7 +93,7 @@ void test_scoped_critical_section2_same_object_no_deadlock(py::class_<BoolWrappe
         barrier.arrive_and_wait();
         py::scoped_critical_section lock{bool_wrapper};
         auto bw = bool_wrapper.cast<std::shared_ptr<BoolWrapper>>();
-        REQUIRE(bw->get() == true);
+        assert(bw->get() == true);
     });
 
     t1.join();

From 543ff87e7f2803b9fd3c8feaf34211a31818c025 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Tue, 3 Jun 2025 17:37:52 +0800
Subject: [PATCH 04/19] feat: enable faulthandler for pytest

---
 tests/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 2cf18c3547..e2139ccf2e 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -567,7 +567,7 @@ set(PYBIND11_PYTEST_ARGS
 # A single command to compile and run the tests
 add_custom_target(
   pytest
-  COMMAND ${PYBIND11_TEST_PREFIX_COMMAND} ${PYTHON_EXECUTABLE} -m pytest
+  COMMAND ${PYBIND11_TEST_PREFIX_COMMAND} ${PYTHON_EXECUTABLE} -X dev -X faulthandler -m pytest
           ${PYBIND11_ABS_PYTEST_FILES} ${PYBIND11_PYTEST_ARGS}
   DEPENDS ${test_targets}
   WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"

From 43e7294fff7b6f0b4ffc79801037a797cea7b90a Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Tue, 3 Jun 2025 18:44:11 +0800
Subject: [PATCH 05/19] chore: use `__has_include(<barrier>)`

---
 tests/test_scoped_critical_section.cpp | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tests/test_scoped_critical_section.cpp b/tests/test_scoped_critical_section.cpp
index efc207693d..1f73e5c1bf 100644
--- a/tests/test_scoped_critical_section.cpp
+++ b/tests/test_scoped_critical_section.cpp
@@ -6,8 +6,10 @@
 #include <cassert>
 #include <thread>
 
-#ifdef PYBIND11_CPP20
+#if defined(__has_include) && __has_include(<barrier>)
+#    define PYBIND11_HAS_BARRIER 1
 #    include <barrier>
+#endif
 
 // Referenced test implementation: https://github.com/PyO3/pyo3/blob/v0.25.0/src/sync.rs#L874
 class BoolWrapper {
@@ -20,6 +22,7 @@ class BoolWrapper {
     std::atomic<bool> value_;
 };
 
+#ifdef PYBIND11_HAS_BARRIER
 void test_scoped_critical_section(py::class_<BoolWrapper> &cls) {
     auto barrier = std::barrier(2);
     auto bool_wrapper = cls(false);
@@ -108,19 +111,15 @@ TEST_SUBMODULE(scoped_critical_section, m) {
 #else
         false;
 #endif
-    m.attr("has_barrier") =
-#if defined(PYBIND11_CPP20)
-        true;
-#else
-        false;
-#endif
 
-#ifdef PYBIND11_CPP20
     auto BoolWrapperClass = py::class_<BoolWrapper>(m, "BoolWrapper")
                                 .def(py::init<bool>())
                                 .def("get", &BoolWrapper::get)
                                 .def("set", &BoolWrapper::set);
 
+#ifdef PYBIND11_HAS_BARRIER
+    m.attr("has_barrier") = true;
+
     m.def("test_scoped_critical_section",
           [&]() -> void { test_scoped_critical_section(BoolWrapperClass); });
     m.def("test_scoped_critical_section2",
@@ -128,5 +127,7 @@ TEST_SUBMODULE(scoped_critical_section, m) {
     m.def("test_scoped_critical_section2_same_object_no_deadlock", [&]() -> void {
         test_scoped_critical_section2_same_object_no_deadlock(BoolWrapperClass);
     });
+#else
+    m.attr("has_barrier") = false;
 #endif
 }

From 3da3d289e33c9f61e87ada4e067990525fa11c69 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Tue, 3 Jun 2025 19:33:23 +0800
Subject: [PATCH 06/19] fix: fix segmentation fault in test

---
 tests/test_scoped_critical_section.cpp | 61 ++++++++++++++++----------
 tests/test_scoped_critical_section.py  |  6 +--
 2 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/tests/test_scoped_critical_section.cpp b/tests/test_scoped_critical_section.cpp
index 1f73e5c1bf..e51f352bd2 100644
--- a/tests/test_scoped_critical_section.cpp
+++ b/tests/test_scoped_critical_section.cpp
@@ -3,10 +3,11 @@
 #include "pybind11_tests.h"
 
 #include <atomic>
-#include <cassert>
+#include <chrono>
 #include <thread>
+#include <utility>
 
-#if defined(__has_include) && __has_include(<barrier>)
+#if defined(PYBIND11_CPP20) && defined(__has_include) && __has_include(<barrier>)
 #    define PYBIND11_HAS_BARRIER 1
 #    include <barrier>
 #endif
@@ -19,18 +20,19 @@ class BoolWrapper {
     void set(bool value) { value_.store(value, std::memory_order_release); }
 
 private:
-    std::atomic<bool> value_;
+    std::atomic<bool> value_{false};
 };
 
 #ifdef PYBIND11_HAS_BARRIER
-void test_scoped_critical_section(py::class_<BoolWrapper> &cls) {
+bool test_scoped_critical_section(const py::handle &cls) {
     auto barrier = std::barrier(2);
     auto bool_wrapper = cls(false);
+    bool output = false;
 
     std::thread t1([&]() {
         py::scoped_critical_section lock{bool_wrapper};
         barrier.arrive_and_wait();
-        auto bw = bool_wrapper.cast<std::shared_ptr<BoolWrapper>>();
+        auto *bw = bool_wrapper.cast<BoolWrapper *>();
         std::this_thread::sleep_for(std::chrono::milliseconds(10));
         bw->set(true);
     });
@@ -38,25 +40,28 @@ void test_scoped_critical_section(py::class_<BoolWrapper> &cls) {
     std::thread t2([&]() {
         barrier.arrive_and_wait();
         py::scoped_critical_section lock{bool_wrapper};
-        auto bw = bool_wrapper.cast<std::shared_ptr<BoolWrapper>>();
-        assert(bw->get() == true);
+        auto *bw = bool_wrapper.cast<BoolWrapper *>();
+        output = bw->get();
     });
 
     t1.join();
     t2.join();
+
+    return output;
 }
 
-void test_scoped_critical_section2(py::class_<BoolWrapper> &cls) {
+std::pair<bool, bool> test_scoped_critical_section2(const py::handle &cls) {
     auto barrier = std::barrier(3);
     auto bool_wrapper1 = cls(false);
     auto bool_wrapper2 = cls(false);
+    std::pair<bool, bool> output{false, false};
 
     std::thread t1([&]() {
         py::scoped_critical_section lock{bool_wrapper1, bool_wrapper2};
         barrier.arrive_and_wait();
         std::this_thread::sleep_for(std::chrono::milliseconds(10));
-        auto bw1 = bool_wrapper1.cast<std::shared_ptr<BoolWrapper>>();
-        auto bw2 = bool_wrapper2.cast<std::shared_ptr<BoolWrapper>>();
+        auto *bw1 = bool_wrapper1.cast<BoolWrapper *>();
+        auto *bw2 = bool_wrapper2.cast<BoolWrapper *>();
         bw1->set(true);
         bw2->set(true);
     });
@@ -64,43 +69,48 @@ void test_scoped_critical_section2(py::class_<BoolWrapper> &cls) {
     std::thread t2([&]() {
         barrier.arrive_and_wait();
         py::scoped_critical_section lock{bool_wrapper1};
-        auto bw1 = bool_wrapper1.cast<std::shared_ptr<BoolWrapper>>();
-        assert(bw1->get() == true);
+        auto *bw1 = bool_wrapper1.cast<BoolWrapper *>();
+        output.first = bw1->get();
     });
 
     std::thread t3([&]() {
         barrier.arrive_and_wait();
         py::scoped_critical_section lock{bool_wrapper2};
-        auto bw2 = bool_wrapper2.cast<std::shared_ptr<BoolWrapper>>();
-        assert(bw2->get() == true);
+        auto *bw2 = bool_wrapper2.cast<BoolWrapper *>();
+        output.second = bw2->get();
     });
 
     t1.join();
     t2.join();
     t3.join();
+
+    return output;
 }
 
-void test_scoped_critical_section2_same_object_no_deadlock(py::class_<BoolWrapper> &cls) {
+bool test_scoped_critical_section2_same_object_no_deadlock(const py::handle &cls) {
     auto barrier = std::barrier(2);
     auto bool_wrapper = cls(false);
+    bool output = false;
 
     std::thread t1([&]() {
         py::scoped_critical_section lock{bool_wrapper, bool_wrapper};
         barrier.arrive_and_wait();
         std::this_thread::sleep_for(std::chrono::milliseconds(10));
-        auto bw = bool_wrapper.cast<std::shared_ptr<BoolWrapper>>();
+        auto *bw = bool_wrapper.cast<BoolWrapper *>();
         bw->set(true);
     });
 
     std::thread t2([&]() {
         barrier.arrive_and_wait();
         py::scoped_critical_section lock{bool_wrapper};
-        auto bw = bool_wrapper.cast<std::shared_ptr<BoolWrapper>>();
-        assert(bw->get() == true);
+        auto *bw = bool_wrapper.cast<BoolWrapper *>();
+        output = bw->get();
     });
 
     t1.join();
     t2.join();
+
+    return output;
 }
 #endif
 
@@ -116,16 +126,19 @@ TEST_SUBMODULE(scoped_critical_section, m) {
                                 .def(py::init<bool>())
                                 .def("get", &BoolWrapper::get)
                                 .def("set", &BoolWrapper::set);
+    auto BoolWrapperHandle = py::handle(BoolWrapperClass);
 
 #ifdef PYBIND11_HAS_BARRIER
     m.attr("has_barrier") = true;
 
-    m.def("test_scoped_critical_section",
-          [&]() -> void { test_scoped_critical_section(BoolWrapperClass); });
-    m.def("test_scoped_critical_section2",
-          [&]() -> void { test_scoped_critical_section2(BoolWrapperClass); });
-    m.def("test_scoped_critical_section2_same_object_no_deadlock", [&]() -> void {
-        test_scoped_critical_section2_same_object_no_deadlock(BoolWrapperClass);
+    m.def("test_scoped_critical_section", [BoolWrapperHandle]() -> bool {
+        return test_scoped_critical_section(BoolWrapperHandle);
+    });
+    m.def("test_scoped_critical_section2", [BoolWrapperHandle]() -> std::pair<bool, bool> {
+        return test_scoped_critical_section2(BoolWrapperHandle);
+    });
+    m.def("test_scoped_critical_section2_same_object_no_deadlock", [BoolWrapperHandle]() -> bool {
+        return test_scoped_critical_section2_same_object_no_deadlock(BoolWrapperHandle);
     });
 #else
     m.attr("has_barrier") = false;
diff --git a/tests/test_scoped_critical_section.py b/tests/test_scoped_critical_section.py
index 49495fc1da..37744c6aec 100644
--- a/tests/test_scoped_critical_section.py
+++ b/tests/test_scoped_critical_section.py
@@ -8,16 +8,16 @@
 @pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
 def test_scoped_critical_section() -> None:
     for _ in range(64):
-        m.test_scoped_critical_section()
+        assert m.test_scoped_critical_section() is True
 
 
 @pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
 def test_scoped_critical_section2() -> None:
     for _ in range(64):
-        assert m.test_scoped_critical_section2()
+        assert m.test_scoped_critical_section2() == (True, True)
 
 
 @pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
 def test_scoped_critical_section2_same_object_no_deadlock() -> None:
     for _ in range(64):
-        m.test_scoped_critical_section2_same_object_no_deadlock()
+        assert m.test_scoped_critical_section2_same_object_no_deadlock() is True

From 5051829a1d99c38f8faa191cfa3a9819f98f1dce Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Tue, 3 Jun 2025 19:40:56 +0800
Subject: [PATCH 07/19] fix: test critical_section for no-gil only

---
 tests/test_scoped_critical_section.cpp | 1 +
 tests/test_scoped_critical_section.py  | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/tests/test_scoped_critical_section.cpp b/tests/test_scoped_critical_section.cpp
index e51f352bd2..59b8e5f99e 100644
--- a/tests/test_scoped_critical_section.cpp
+++ b/tests/test_scoped_critical_section.cpp
@@ -127,6 +127,7 @@ TEST_SUBMODULE(scoped_critical_section, m) {
                                 .def("get", &BoolWrapper::get)
                                 .def("set", &BoolWrapper::set);
     auto BoolWrapperHandle = py::handle(BoolWrapperClass);
+    (void) BoolWrapperHandle.ptr(); // suppress unused variable warning
 
 #ifdef PYBIND11_HAS_BARRIER
     m.attr("has_barrier") = true;
diff --git a/tests/test_scoped_critical_section.py b/tests/test_scoped_critical_section.py
index 37744c6aec..6cfd339498 100644
--- a/tests/test_scoped_critical_section.py
+++ b/tests/test_scoped_critical_section.py
@@ -2,21 +2,25 @@
 
 import pytest
 
+import env
 from pybind11_tests import scoped_critical_section as m
 
 
+@pytest.mark.skipif(not env.PY_GIL_DISABLED, reason="requires GIL disabled")
 @pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
 def test_scoped_critical_section() -> None:
     for _ in range(64):
         assert m.test_scoped_critical_section() is True
 
 
+@pytest.mark.skipif(not env.PY_GIL_DISABLED, reason="requires GIL disabled")
 @pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
 def test_scoped_critical_section2() -> None:
     for _ in range(64):
         assert m.test_scoped_critical_section2() == (True, True)
 
 
+@pytest.mark.skipif(not env.PY_GIL_DISABLED, reason="requires GIL disabled")
 @pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
 def test_scoped_critical_section2_same_object_no_deadlock() -> None:
     for _ in range(64):

From adbc2b2492667526878b7b0b235dbc399b6d9dc9 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Tue, 3 Jun 2025 20:21:59 +0800
Subject: [PATCH 08/19] test: run new tests only

---
 .github/workflows/ci.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 9e8d3b6fe5..9c7ca822fe 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -20,6 +20,7 @@ env:
   PIP_ONLY_BINARY: numpy
   FORCE_COLOR: 3
   PYTEST_TIMEOUT: 300
+  PYTEST_ADDOPTS: "-k critical_section"
   # For cmake:
   VERBOSE: 1
   CMAKE_COLOR_DIAGNOSTICS: 1

From 11e037b85e8f7f11bff6c4e1a9c9993f7332bbed Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Tue, 3 Jun 2025 20:39:29 +0800
Subject: [PATCH 09/19] test: ensure non-empty test selection

---
 tests/test_scoped_critical_section.cpp | 44 +++++++++++++++++---------
 tests/test_scoped_critical_section.py  | 10 ++----
 2 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/tests/test_scoped_critical_section.cpp b/tests/test_scoped_critical_section.cpp
index 59b8e5f99e..cae94bd522 100644
--- a/tests/test_scoped_critical_section.cpp
+++ b/tests/test_scoped_critical_section.cpp
@@ -23,8 +23,9 @@ class BoolWrapper {
     std::atomic<bool> value_{false};
 };
 
-#ifdef PYBIND11_HAS_BARRIER
-bool test_scoped_critical_section(const py::handle &cls) {
+#if defined(PYBIND11_HAS_BARRIER) && defined(Py_GIL_DISABLED)
+
+void test_scoped_critical_section(const py::handle &cls) {
     auto barrier = std::barrier(2);
     auto bool_wrapper = cls(false);
     bool output = false;
@@ -47,10 +48,12 @@ bool test_scoped_critical_section(const py::handle &cls) {
     t1.join();
     t2.join();
 
-    return output;
+    if (!output) {
+        throw std::runtime_error("Scoped critical section test failed: output is false");
+    }
 }
 
-std::pair<bool, bool> test_scoped_critical_section2(const py::handle &cls) {
+void test_scoped_critical_section2(const py::handle &cls) {
     auto barrier = std::barrier(3);
     auto bool_wrapper1 = cls(false);
     auto bool_wrapper2 = cls(false);
@@ -84,10 +87,13 @@ std::pair<bool, bool> test_scoped_critical_section2(const py::handle &cls) {
     t2.join();
     t3.join();
 
-    return output;
+    if (!output.first || !output.second) {
+        throw std::runtime_error(
+            "Scoped critical section test with two objects failed: output is false");
+    }
 }
 
-bool test_scoped_critical_section2_same_object_no_deadlock(const py::handle &cls) {
+void test_scoped_critical_section2_same_object_no_deadlock(const py::handle &cls) {
     auto barrier = std::barrier(2);
     auto bool_wrapper = cls(false);
     bool output = false;
@@ -110,8 +116,18 @@ bool test_scoped_critical_section2_same_object_no_deadlock(const py::handle &cls
     t1.join();
     t2.join();
 
-    return output;
+    if (!output) {
+        throw std::runtime_error(
+            "Scoped critical section test with same object failed: output is false");
+    }
 }
+
+#else
+
+void test_scoped_critical_section(const py::handle &) {}
+void test_scoped_critical_section2(const py::handle &) {}
+void test_scoped_critical_section2_same_object_no_deadlock(const py::handle &) {}
+
 #endif
 
 TEST_SUBMODULE(scoped_critical_section, m) {
@@ -132,14 +148,12 @@ TEST_SUBMODULE(scoped_critical_section, m) {
 #ifdef PYBIND11_HAS_BARRIER
     m.attr("has_barrier") = true;
 
-    m.def("test_scoped_critical_section", [BoolWrapperHandle]() -> bool {
-        return test_scoped_critical_section(BoolWrapperHandle);
-    });
-    m.def("test_scoped_critical_section2", [BoolWrapperHandle]() -> std::pair<bool, bool> {
-        return test_scoped_critical_section2(BoolWrapperHandle);
-    });
-    m.def("test_scoped_critical_section2_same_object_no_deadlock", [BoolWrapperHandle]() -> bool {
-        return test_scoped_critical_section2_same_object_no_deadlock(BoolWrapperHandle);
+    m.def("test_scoped_critical_section",
+          [BoolWrapperHandle]() -> void { test_scoped_critical_section(BoolWrapperHandle); });
+    m.def("test_scoped_critical_section2",
+          [BoolWrapperHandle]() -> void { test_scoped_critical_section2(BoolWrapperHandle); });
+    m.def("test_scoped_critical_section2_same_object_no_deadlock", [BoolWrapperHandle]() -> void {
+        test_scoped_critical_section2_same_object_no_deadlock(BoolWrapperHandle);
     });
 #else
     m.attr("has_barrier") = false;
diff --git a/tests/test_scoped_critical_section.py b/tests/test_scoped_critical_section.py
index 6cfd339498..579192594e 100644
--- a/tests/test_scoped_critical_section.py
+++ b/tests/test_scoped_critical_section.py
@@ -2,26 +2,22 @@
 
 import pytest
 
-import env
 from pybind11_tests import scoped_critical_section as m
 
 
-@pytest.mark.skipif(not env.PY_GIL_DISABLED, reason="requires GIL disabled")
 @pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
 def test_scoped_critical_section() -> None:
     for _ in range(64):
-        assert m.test_scoped_critical_section() is True
+        m.test_scoped_critical_section()
 
 
-@pytest.mark.skipif(not env.PY_GIL_DISABLED, reason="requires GIL disabled")
 @pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
 def test_scoped_critical_section2() -> None:
     for _ in range(64):
-        assert m.test_scoped_critical_section2() == (True, True)
+        m.test_scoped_critical_section2()
 
 
-@pytest.mark.skipif(not env.PY_GIL_DISABLED, reason="requires GIL disabled")
 @pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
 def test_scoped_critical_section2_same_object_no_deadlock() -> None:
     for _ in range(64):
-        assert m.test_scoped_critical_section2_same_object_no_deadlock() is True
+        m.test_scoped_critical_section2_same_object_no_deadlock()

From e2e11fdc090873a8a5234c664a50cfac8c93d797 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Tue, 3 Jun 2025 22:39:56 +0800
Subject: [PATCH 10/19] fix: fix test critical_section

---
 .github/workflows/ci.yml               |   1 -
 tests/test_scoped_critical_section.cpp | 151 +++++++++++++++----------
 2 files changed, 89 insertions(+), 63 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 9c7ca822fe..9e8d3b6fe5 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -20,7 +20,6 @@ env:
   PIP_ONLY_BINARY: numpy
   FORCE_COLOR: 3
   PYTEST_TIMEOUT: 300
-  PYTEST_ADDOPTS: "-k critical_section"
   # For cmake:
   VERBOSE: 1
   CMAKE_COLOR_DIAGNOSTICS: 1
diff --git a/tests/test_scoped_critical_section.cpp b/tests/test_scoped_critical_section.cpp
index cae94bd522..5878b33b7b 100644
--- a/tests/test_scoped_critical_section.cpp
+++ b/tests/test_scoped_critical_section.cpp
@@ -23,30 +23,38 @@ class BoolWrapper {
     std::atomic<bool> value_{false};
 };
 
-#if defined(PYBIND11_HAS_BARRIER) && defined(Py_GIL_DISABLED)
+#if defined(PYBIND11_HAS_BARRIER)
 
 void test_scoped_critical_section(const py::handle &cls) {
     auto barrier = std::barrier(2);
     auto bool_wrapper = cls(false);
     bool output = false;
 
-    std::thread t1([&]() {
-        py::scoped_critical_section lock{bool_wrapper};
-        barrier.arrive_and_wait();
-        auto *bw = bool_wrapper.cast<BoolWrapper *>();
-        std::this_thread::sleep_for(std::chrono::milliseconds(10));
-        bw->set(true);
-    });
-
-    std::thread t2([&]() {
-        barrier.arrive_and_wait();
-        py::scoped_critical_section lock{bool_wrapper};
-        auto *bw = bool_wrapper.cast<BoolWrapper *>();
-        output = bw->get();
-    });
-
-    t1.join();
-    t2.join();
+    {
+        py::gil_scoped_release gil_release{};
+
+        std::thread t1([&]() {
+            py::gil_scoped_acquire ensure_tstate{};
+            py::scoped_critical_section lock{bool_wrapper};
+            barrier.arrive_and_wait();
+            auto *bw = bool_wrapper.cast<BoolWrapper *>();
+            std::this_thread::sleep_for(std::chrono::milliseconds(10));
+            bw->set(true);
+        });
+
+        std::thread t2([&]() {
+            barrier.arrive_and_wait();
+            {
+                py::gil_scoped_acquire ensure_tstate{};
+                py::scoped_critical_section lock{bool_wrapper};
+                auto *bw = bool_wrapper.cast<BoolWrapper *>();
+                output = bw->get();
+            }
+        });
+
+        t1.join();
+        t2.join();
+    }
 
     if (!output) {
         throw std::runtime_error("Scoped critical section test failed: output is false");
@@ -59,33 +67,44 @@ void test_scoped_critical_section2(const py::handle &cls) {
     auto bool_wrapper2 = cls(false);
     std::pair<bool, bool> output{false, false};
 
-    std::thread t1([&]() {
-        py::scoped_critical_section lock{bool_wrapper1, bool_wrapper2};
-        barrier.arrive_and_wait();
-        std::this_thread::sleep_for(std::chrono::milliseconds(10));
-        auto *bw1 = bool_wrapper1.cast<BoolWrapper *>();
-        auto *bw2 = bool_wrapper2.cast<BoolWrapper *>();
-        bw1->set(true);
-        bw2->set(true);
-    });
-
-    std::thread t2([&]() {
-        barrier.arrive_and_wait();
-        py::scoped_critical_section lock{bool_wrapper1};
-        auto *bw1 = bool_wrapper1.cast<BoolWrapper *>();
-        output.first = bw1->get();
-    });
-
-    std::thread t3([&]() {
-        barrier.arrive_and_wait();
-        py::scoped_critical_section lock{bool_wrapper2};
-        auto *bw2 = bool_wrapper2.cast<BoolWrapper *>();
-        output.second = bw2->get();
-    });
-
-    t1.join();
-    t2.join();
-    t3.join();
+    {
+        py::gil_scoped_release gil_release{};
+
+        std::thread t1([&]() {
+            py::gil_scoped_acquire ensure_tstate{};
+            py::scoped_critical_section lock{bool_wrapper1, bool_wrapper2};
+            barrier.arrive_and_wait();
+            std::this_thread::sleep_for(std::chrono::milliseconds(10));
+            auto *bw1 = bool_wrapper1.cast<BoolWrapper *>();
+            auto *bw2 = bool_wrapper2.cast<BoolWrapper *>();
+            bw1->set(true);
+            bw2->set(true);
+        });
+
+        std::thread t2([&]() {
+            barrier.arrive_and_wait();
+            {
+                py::gil_scoped_acquire ensure_tstate{};
+                py::scoped_critical_section lock{bool_wrapper1};
+                auto *bw1 = bool_wrapper1.cast<BoolWrapper *>();
+                output.first = bw1->get();
+            }
+        });
+
+        std::thread t3([&]() {
+            barrier.arrive_and_wait();
+            {
+                py::gil_scoped_acquire ensure_tstate{};
+                py::scoped_critical_section lock{bool_wrapper2};
+                auto *bw2 = bool_wrapper2.cast<BoolWrapper *>();
+                output.second = bw2->get();
+            }
+        });
+
+        t1.join();
+        t2.join();
+        t3.join();
+    }
 
     if (!output.first || !output.second) {
         throw std::runtime_error(
@@ -98,23 +117,31 @@ void test_scoped_critical_section2_same_object_no_deadlock(const py::handle &cls
     auto bool_wrapper = cls(false);
     bool output = false;
 
-    std::thread t1([&]() {
-        py::scoped_critical_section lock{bool_wrapper, bool_wrapper};
-        barrier.arrive_and_wait();
-        std::this_thread::sleep_for(std::chrono::milliseconds(10));
-        auto *bw = bool_wrapper.cast<BoolWrapper *>();
-        bw->set(true);
-    });
-
-    std::thread t2([&]() {
-        barrier.arrive_and_wait();
-        py::scoped_critical_section lock{bool_wrapper};
-        auto *bw = bool_wrapper.cast<BoolWrapper *>();
-        output = bw->get();
-    });
-
-    t1.join();
-    t2.join();
+    {
+        py::gil_scoped_release gil_release{};
+
+        std::thread t1([&]() {
+            py::gil_scoped_acquire ensure_tstate{};
+            py::scoped_critical_section lock{bool_wrapper, bool_wrapper};
+            barrier.arrive_and_wait();
+            std::this_thread::sleep_for(std::chrono::milliseconds(10));
+            auto *bw = bool_wrapper.cast<BoolWrapper *>();
+            bw->set(true);
+        });
+
+        std::thread t2([&]() {
+            barrier.arrive_and_wait();
+            {
+                py::gil_scoped_acquire ensure_tstate{};
+                py::scoped_critical_section lock{bool_wrapper};
+                auto *bw = bool_wrapper.cast<BoolWrapper *>();
+                output = bw->get();
+            }
+        });
+
+        t1.join();
+        t2.join();
+    }
 
     if (!output) {
         throw std::runtime_error(

From 39d0aa4eb439a7c7481017c869905fb0ac917c7e Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Wed, 4 Jun 2025 02:10:11 +0800
Subject: [PATCH 11/19] fix: change Python 3.14.0b1/2 xfail tests to non-strict

---
 tests/test_methods_and_attributes.py | 2 +-
 tests/test_pickling.py               | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test_methods_and_attributes.py b/tests/test_methods_and_attributes.py
index cc3a51e0d0..143e573afd 100644
--- a/tests/test_methods_and_attributes.py
+++ b/tests/test_methods_and_attributes.py
@@ -309,7 +309,7 @@ def test_property_rvalue_policy():
     sys.version_info == (3, 14, 0, "beta", 1)
     or sys.version_info == (3, 14, 0, "beta", 2),
     reason="3.14.0b1/2 bug: https://github.com/python/cpython/issues/133912",
-    strict=True,
+    strict=False,
 )
 def test_dynamic_attributes():
     instance = m.DynamicClass()
diff --git a/tests/test_pickling.py b/tests/test_pickling.py
index 56f91cc4bb..94cbb7b3dd 100644
--- a/tests/test_pickling.py
+++ b/tests/test_pickling.py
@@ -72,7 +72,7 @@ def test_roundtrip(cls_name):
                 sys.version_info == (3, 14, 0, "beta", 1)
                 or sys.version_info == (3, 14, 0, "beta", 2),
                 reason="3.14.0b1/2 bug: https://github.com/python/cpython/issues/133912",
-                strict=True,
+                strict=False,
             ),
         ),
         "PickleableWithDictNew",

From 8b48a0c68491f19e9ed95543364c9774891f8920 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Wed, 4 Jun 2025 02:51:43 +0800
Subject: [PATCH 12/19] test: trigger gc manually

---
 tests/test_methods_and_attributes.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/test_methods_and_attributes.py b/tests/test_methods_and_attributes.py
index 143e573afd..507b4c0c6a 100644
--- a/tests/test_methods_and_attributes.py
+++ b/tests/test_methods_and_attributes.py
@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import gc
 import sys
 
 import pytest
@@ -19,6 +20,12 @@
 )
 
 
+def gc_collect(repeat=5):
+    """Collect garbage multiple times to ensure that objects are actually deleted."""
+    for _ in range(repeat):
+        gc.collect()
+
+
 def test_self_only_pos_only():
     assert (
         m.ExampleMandA.__str__.__doc__
@@ -337,19 +344,21 @@ def test_dynamic_attributes():
     cstats = ConstructorStats.get(m.DynamicClass)
     assert cstats.alive() == 1
     del instance
+    gc_collect(repeat=10)
     assert cstats.alive() == 0
 
     # Derived classes should work as well
     class PythonDerivedDynamicClass(m.DynamicClass):
         pass
 
-    for cls in m.CppDerivedDynamicClass, PythonDerivedDynamicClass:
+    for cls in (m.CppDerivedDynamicClass, PythonDerivedDynamicClass):
         derived = cls()
         derived.foobar = 100
         assert derived.foobar == 100
 
         assert cstats.alive() == 1
         del derived
+        gc_collect(repeat=10)
         assert cstats.alive() == 0
 
 
@@ -364,6 +373,7 @@ def test_cyclic_gc():
     cstats = ConstructorStats.get(m.DynamicClass)
     assert cstats.alive() == 1
     del instance
+    gc_collect(repeat=10)
     assert cstats.alive() == 0
 
     # Two object reference each other
@@ -374,6 +384,7 @@ def test_cyclic_gc():
 
     assert cstats.alive() == 2
     del i1, i2
+    gc_collect(repeat=10)
     assert cstats.alive() == 0
 
 

From 009756f1e90163f111444b31934fd661b13c4bf4 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Wed, 4 Jun 2025 03:08:38 +0800
Subject: [PATCH 13/19] test: mark xfail to `DynamicClass`

---
 tests/test_methods_and_attributes.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/test_methods_and_attributes.py b/tests/test_methods_and_attributes.py
index 507b4c0c6a..4b39817d79 100644
--- a/tests/test_methods_and_attributes.py
+++ b/tests/test_methods_and_attributes.py
@@ -364,6 +364,12 @@ class PythonDerivedDynamicClass(m.DynamicClass):
 
 # https://foss.heptapod.net/pypy/pypy/-/issues/2447
 @pytest.mark.xfail("env.PYPY")
+@pytest.mark.xfail(
+    sys.version_info == (3, 14, 0, "beta", 1)
+    or sys.version_info == (3, 14, 0, "beta", 2),
+    reason="3.14.0b1/2 bug: https://github.com/python/cpython/issues/133912",
+    strict=False,
+)
 @pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC")
 def test_cyclic_gc():
     # One object references itself

From 58f155c5b5975f697429094fed62e34efc1fbbba Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rgrossekunst@nvidia.com>
Date: Tue, 3 Jun 2025 21:20:38 -0700
Subject: [PATCH 14/19] Use `namespace test_scoped_critical_section_ns`
 (standard approach to guard against name clashes).

---
 tests/test_scoped_critical_section.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/test_scoped_critical_section.cpp b/tests/test_scoped_critical_section.cpp
index 5878b33b7b..54494ab4ff 100644
--- a/tests/test_scoped_critical_section.cpp
+++ b/tests/test_scoped_critical_section.cpp
@@ -12,6 +12,8 @@
 #    include <barrier>
 #endif
 
+namespace test_scoped_critical_section_ns {
+
 // Referenced test implementation: https://github.com/PyO3/pyo3/blob/v0.25.0/src/sync.rs#L874
 class BoolWrapper {
 public:
@@ -157,7 +159,11 @@ void test_scoped_critical_section2_same_object_no_deadlock(const py::handle &) {
 
 #endif
 
+} // namespace test_scoped_critical_section_ns
+
 TEST_SUBMODULE(scoped_critical_section, m) {
+    using namespace test_scoped_critical_section_ns;
+
     m.attr("defined_THREAD_SANITIZER") =
 #if defined(THREAD_SANITIZER)
         true;

From 2ce1f8a9de24e6a1e5e1d798f80d1dda247bdee3 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rgrossekunst@nvidia.com>
Date: Tue, 3 Jun 2025 22:03:41 -0700
Subject: [PATCH 15/19] Simplify changes in pybind11/critical_section.h and add
 test_nullptr_combinations()

---
 include/pybind11/critical_section.h    | 36 ++++++++++++++------------
 tests/test_scoped_critical_section.cpp | 24 ++++++++++++-----
 tests/test_scoped_critical_section.py  |  7 +++++
 3 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/include/pybind11/critical_section.h b/include/pybind11/critical_section.h
index cbff504692..dd5bac9355 100644
--- a/include/pybind11/critical_section.h
+++ b/include/pybind11/critical_section.h
@@ -13,28 +13,33 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 class scoped_critical_section {
 public:
 #ifdef Py_GIL_DISABLED
-    scoped_critical_section(handle obj1, handle obj2) : m_ptr1(obj1.ptr()), m_ptr2(obj2.ptr()) {
-        if (m_ptr1 == nullptr) {
-            std::swap(m_ptr1, m_ptr2);
-        }
-        if (m_ptr2 != nullptr) {
-            PyCriticalSection2_Begin(&section2, m_ptr1, m_ptr2);
-        } else if (m_ptr1 != nullptr) {
-            PyCriticalSection_Begin(&section, m_ptr1);
+    scoped_critical_section(handle obj1, handle obj2) {
+        if (obj1) {
+            if (obj2) {
+                PyCriticalSection2_Begin(&section2, obj1.ptr(), obj2.ptr());
+                rank = 2;
+            } else {
+                PyCriticalSection_Begin(&section, obj1.ptr());
+                rank = 1;
+            }
+        } else if (obj2) {
+            PyCriticalSection_Begin(&section, obj2.ptr());
+            rank = 1;
         }
     }
 
-    explicit scoped_critical_section(handle obj) : m_ptr1(obj.ptr()) {
-        if (m_ptr1 != nullptr) {
-            PyCriticalSection_Begin(&section, m_ptr1);
+    explicit scoped_critical_section(handle obj) {
+        if (obj) {
+            PyCriticalSection_Begin(&section, obj.ptr());
+            rank = 1;
         }
     }
 
     ~scoped_critical_section() {
-        if (m_ptr2 != nullptr) {
-            PyCriticalSection2_End(&section2);
-        } else if (m_ptr1 != nullptr) {
+        if (rank == 1) {
             PyCriticalSection_End(&section);
+        } else if (rank == 2) {
+            PyCriticalSection2_End(&section2);
         }
     }
 #else
@@ -48,8 +53,7 @@ class scoped_critical_section {
 
 private:
 #ifdef Py_GIL_DISABLED
-    PyObject *m_ptr1{nullptr};
-    PyObject *m_ptr2{nullptr};
+    int rank{0};
     union {
         PyCriticalSection section;
         PyCriticalSection2 section2;
diff --git a/tests/test_scoped_critical_section.cpp b/tests/test_scoped_critical_section.cpp
index 54494ab4ff..3af9ae01db 100644
--- a/tests/test_scoped_critical_section.cpp
+++ b/tests/test_scoped_critical_section.cpp
@@ -14,6 +14,20 @@
 
 namespace test_scoped_critical_section_ns {
 
+void test_one_nullptr() { py::scoped_critical_section lock{py::handle{}}; }
+
+void test_two_nullptrs() { py::scoped_critical_section lock{py::handle{}, py::handle{}}; }
+
+void test_first_nullptr() {
+    py::dict d;
+    py::scoped_critical_section lock{py::handle{}, d};
+}
+
+void test_second_nullptr() {
+    py::dict d;
+    py::scoped_critical_section lock{d, py::handle{}};
+}
+
 // Referenced test implementation: https://github.com/PyO3/pyo3/blob/v0.25.0/src/sync.rs#L874
 class BoolWrapper {
 public:
@@ -164,12 +178,10 @@ void test_scoped_critical_section2_same_object_no_deadlock(const py::handle &) {
 TEST_SUBMODULE(scoped_critical_section, m) {
     using namespace test_scoped_critical_section_ns;
 
-    m.attr("defined_THREAD_SANITIZER") =
-#if defined(THREAD_SANITIZER)
-        true;
-#else
-        false;
-#endif
+    m.def("test_one_nullptr", test_one_nullptr);
+    m.def("test_two_nullptrs", test_two_nullptrs);
+    m.def("test_first_nullptr", test_first_nullptr);
+    m.def("test_second_nullptr", test_second_nullptr);
 
     auto BoolWrapperClass = py::class_<BoolWrapper>(m, "BoolWrapper")
                                 .def(py::init<bool>())
diff --git a/tests/test_scoped_critical_section.py b/tests/test_scoped_critical_section.py
index 579192594e..5703e3d51c 100644
--- a/tests/test_scoped_critical_section.py
+++ b/tests/test_scoped_critical_section.py
@@ -5,6 +5,13 @@
 from pybind11_tests import scoped_critical_section as m
 
 
+def test_nullptr_combinations():
+    m.test_one_nullptr()
+    m.test_two_nullptrs()
+    m.test_first_nullptr()
+    m.test_second_nullptr()
+
+
 @pytest.mark.skipif(not m.has_barrier, reason="no <barrier>")
 def test_scoped_critical_section() -> None:
     for _ in range(64):

From ad4203f92ccf17ba7a4ffaf495023bcd736a6644 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Wed, 4 Jun 2025 13:35:06 +0800
Subject: [PATCH 16/19] test: disable Python devmode in pytest

---
 tests/CMakeLists.txt                 |  2 +-
 tests/test_methods_and_attributes.py | 21 ++-------------------
 tests/test_pickling.py               |  2 +-
 3 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index e2139ccf2e..2cf18c3547 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -567,7 +567,7 @@ set(PYBIND11_PYTEST_ARGS
 # A single command to compile and run the tests
 add_custom_target(
   pytest
-  COMMAND ${PYBIND11_TEST_PREFIX_COMMAND} ${PYTHON_EXECUTABLE} -X dev -X faulthandler -m pytest
+  COMMAND ${PYBIND11_TEST_PREFIX_COMMAND} ${PYTHON_EXECUTABLE} -m pytest
           ${PYBIND11_ABS_PYTEST_FILES} ${PYBIND11_PYTEST_ARGS}
   DEPENDS ${test_targets}
   WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
diff --git a/tests/test_methods_and_attributes.py b/tests/test_methods_and_attributes.py
index 4b39817d79..cc3a51e0d0 100644
--- a/tests/test_methods_and_attributes.py
+++ b/tests/test_methods_and_attributes.py
@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import gc
 import sys
 
 import pytest
@@ -20,12 +19,6 @@
 )
 
 
-def gc_collect(repeat=5):
-    """Collect garbage multiple times to ensure that objects are actually deleted."""
-    for _ in range(repeat):
-        gc.collect()
-
-
 def test_self_only_pos_only():
     assert (
         m.ExampleMandA.__str__.__doc__
@@ -316,7 +309,7 @@ def test_property_rvalue_policy():
     sys.version_info == (3, 14, 0, "beta", 1)
     or sys.version_info == (3, 14, 0, "beta", 2),
     reason="3.14.0b1/2 bug: https://github.com/python/cpython/issues/133912",
-    strict=False,
+    strict=True,
 )
 def test_dynamic_attributes():
     instance = m.DynamicClass()
@@ -344,32 +337,24 @@ def test_dynamic_attributes():
     cstats = ConstructorStats.get(m.DynamicClass)
     assert cstats.alive() == 1
     del instance
-    gc_collect(repeat=10)
     assert cstats.alive() == 0
 
     # Derived classes should work as well
     class PythonDerivedDynamicClass(m.DynamicClass):
         pass
 
-    for cls in (m.CppDerivedDynamicClass, PythonDerivedDynamicClass):
+    for cls in m.CppDerivedDynamicClass, PythonDerivedDynamicClass:
         derived = cls()
         derived.foobar = 100
         assert derived.foobar == 100
 
         assert cstats.alive() == 1
         del derived
-        gc_collect(repeat=10)
         assert cstats.alive() == 0
 
 
 # https://foss.heptapod.net/pypy/pypy/-/issues/2447
 @pytest.mark.xfail("env.PYPY")
-@pytest.mark.xfail(
-    sys.version_info == (3, 14, 0, "beta", 1)
-    or sys.version_info == (3, 14, 0, "beta", 2),
-    reason="3.14.0b1/2 bug: https://github.com/python/cpython/issues/133912",
-    strict=False,
-)
 @pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC")
 def test_cyclic_gc():
     # One object references itself
@@ -379,7 +364,6 @@ def test_cyclic_gc():
     cstats = ConstructorStats.get(m.DynamicClass)
     assert cstats.alive() == 1
     del instance
-    gc_collect(repeat=10)
     assert cstats.alive() == 0
 
     # Two object reference each other
@@ -390,7 +374,6 @@ def test_cyclic_gc():
 
     assert cstats.alive() == 2
     del i1, i2
-    gc_collect(repeat=10)
     assert cstats.alive() == 0
 
 
diff --git a/tests/test_pickling.py b/tests/test_pickling.py
index 94cbb7b3dd..56f91cc4bb 100644
--- a/tests/test_pickling.py
+++ b/tests/test_pickling.py
@@ -72,7 +72,7 @@ def test_roundtrip(cls_name):
                 sys.version_info == (3, 14, 0, "beta", 1)
                 or sys.version_info == (3, 14, 0, "beta", 2),
                 reason="3.14.0b1/2 bug: https://github.com/python/cpython/issues/133912",
-                strict=False,
+                strict=True,
             ),
         ),
         "PickleableWithDictNew",

From 883ac69cb3a939eb1b1a196a1861e1e49cb39165 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Wed, 4 Jun 2025 15:03:13 +0800
Subject: [PATCH 17/19] test: add comprehensive comments for the tests

---
 tests/test_scoped_critical_section.cpp | 78 +++++++++++++++++++++++---
 1 file changed, 71 insertions(+), 7 deletions(-)

diff --git a/tests/test_scoped_critical_section.cpp b/tests/test_scoped_critical_section.cpp
index 3af9ae01db..7833e9c885 100644
--- a/tests/test_scoped_critical_section.cpp
+++ b/tests/test_scoped_critical_section.cpp
@@ -36,7 +36,7 @@ class BoolWrapper {
     void set(bool value) { value_.store(value, std::memory_order_release); }
 
 private:
-    std::atomic<bool> value_{false};
+    std::atomic_bool value_{false};
 };
 
 #if defined(PYBIND11_HAS_BARRIER)
@@ -47,22 +47,40 @@ void test_scoped_critical_section(const py::handle &cls) {
     bool output = false;
 
     {
+        // Release the GIL to allow run threads in parallel.
         py::gil_scoped_release gil_release{};
 
         std::thread t1([&]() {
+            // Use gil_scoped_acquire to ensure we have a valid Python thread state
+            // before entering the critical section. Otherwise, the critical section
+            // will cause a segmentation fault.
             py::gil_scoped_acquire ensure_tstate{};
+            // Enter the critical section with the same object as the second thread.
             py::scoped_critical_section lock{bool_wrapper};
+            // At this point, the object is locked by this thread via the scoped_critical_section.
+            // This barrier will ensure that the second thread waits until this thread has released
+            // the critical section before proceeding.
             barrier.arrive_and_wait();
-            auto *bw = bool_wrapper.cast<BoolWrapper *>();
+            // Sleep for a short time to simulate some work in the critical section.
+            // This sleep is necessary to test the locking mechanism properly.
             std::this_thread::sleep_for(std::chrono::milliseconds(10));
+            auto *bw = bool_wrapper.cast<BoolWrapper *>();
             bw->set(true);
         });
 
         std::thread t2([&]() {
+            // This thread will wait until the first thread has entered the critical section due to
+            // the barrier.
             barrier.arrive_and_wait();
             {
+                // Use gil_scoped_acquire to ensure we have a valid Python thread state
+                // before entering the critical section. Otherwise, the critical section
+                // will cause a segmentation fault.
                 py::gil_scoped_acquire ensure_tstate{};
+                // Enter the critical section with the same object as the first thread.
                 py::scoped_critical_section lock{bool_wrapper};
+                // At this point, the critical section is released by the first thread, the value
+                // is set to true.
                 auto *bw = bool_wrapper.cast<BoolWrapper *>();
                 output = bw->get();
             }
@@ -84,12 +102,23 @@ void test_scoped_critical_section2(const py::handle &cls) {
     std::pair<bool, bool> output{false, false};
 
     {
+        // Release the GIL to allow run threads in parallel.
         py::gil_scoped_release gil_release{};
 
         std::thread t1([&]() {
+            // Use gil_scoped_acquire to ensure we have a valid Python thread state
+            // before entering the critical section. Otherwise, the critical section
+            // will cause a segmentation fault.
             py::gil_scoped_acquire ensure_tstate{};
+            // Enter the critical section with two different objects.
+            // This will ensure that the critical section is locked for both objects.
             py::scoped_critical_section lock{bool_wrapper1, bool_wrapper2};
+            // At this point, objects is locked by this thread via the scoped_critical_section.
+            // This barrier will ensure that other threads wait until this thread has released
+            // the critical section before proceeding.
             barrier.arrive_and_wait();
+            // Sleep for a short time to simulate some work in the critical section.
+            // This sleep is necessary to test the locking mechanism properly.
             std::this_thread::sleep_for(std::chrono::milliseconds(10));
             auto *bw1 = bool_wrapper1.cast<BoolWrapper *>();
             auto *bw2 = bool_wrapper2.cast<BoolWrapper *>();
@@ -98,20 +127,36 @@ void test_scoped_critical_section2(const py::handle &cls) {
         });
 
         std::thread t2([&]() {
+            // This thread will wait until the first thread has entered the critical section due to
+            // the barrier.
             barrier.arrive_and_wait();
             {
+                // Use gil_scoped_acquire to ensure we have a valid Python thread state
+                // before entering the critical section. Otherwise, the critical section
+                // will cause a segmentation fault.
                 py::gil_scoped_acquire ensure_tstate{};
+                // Enter the critical section with the same object as the first thread.
                 py::scoped_critical_section lock{bool_wrapper1};
+                // At this point, the critical section is released by the first thread, the value
+                // is set to true.
                 auto *bw1 = bool_wrapper1.cast<BoolWrapper *>();
                 output.first = bw1->get();
             }
         });
 
         std::thread t3([&]() {
+            // This thread will wait until the first thread has entered the critical section due to
+            // the barrier.
             barrier.arrive_and_wait();
             {
+                // Use gil_scoped_acquire to ensure we have a valid Python thread state
+                // before entering the critical section. Otherwise, the critical section
+                // will cause a segmentation fault.
                 py::gil_scoped_acquire ensure_tstate{};
+                // Enter the critical section with the same object as the first thread.
                 py::scoped_critical_section lock{bool_wrapper2};
+                // At this point, the critical section is released by the first thread, the value
+                // is set to true.
                 auto *bw2 = bool_wrapper2.cast<BoolWrapper *>();
                 output.second = bw2->get();
             }
@@ -134,22 +179,40 @@ void test_scoped_critical_section2_same_object_no_deadlock(const py::handle &cls
     bool output = false;
 
     {
+        // Release the GIL to allow run threads in parallel.
         py::gil_scoped_release gil_release{};
 
         std::thread t1([&]() {
+            // Use gil_scoped_acquire to ensure we have a valid Python thread state
+            // before entering the critical section. Otherwise, the critical section
+            // will cause a segmentation fault.
             py::gil_scoped_acquire ensure_tstate{};
-            py::scoped_critical_section lock{bool_wrapper, bool_wrapper};
+            // Enter the critical section with the same object as the second thread.
+            py::scoped_critical_section lock{bool_wrapper, bool_wrapper}; // same object used here
+            // At this point, the object is locked by this thread via the scoped_critical_section.
+            // This barrier will ensure that the second thread waits until this thread has released
+            // the critical section before proceeding.
             barrier.arrive_and_wait();
+            // Sleep for a short time to simulate some work in the critical section.
+            // This sleep is necessary to test the locking mechanism properly.
             std::this_thread::sleep_for(std::chrono::milliseconds(10));
             auto *bw = bool_wrapper.cast<BoolWrapper *>();
             bw->set(true);
         });
 
         std::thread t2([&]() {
+            // This thread will wait until the first thread has entered the critical section due to
+            // the barrier.
             barrier.arrive_and_wait();
             {
+                // Use gil_scoped_acquire to ensure we have a valid Python thread state
+                // before entering the critical section. Otherwise, the critical section
+                // will cause a segmentation fault.
                 py::gil_scoped_acquire ensure_tstate{};
+                // Enter the critical section with the same object as the first thread.
                 py::scoped_critical_section lock{bool_wrapper};
+                // At this point, the critical section is released by the first thread, the value
+                // is set to true.
                 auto *bw = bool_wrapper.cast<BoolWrapper *>();
                 output = bw->get();
             }
@@ -190,8 +253,12 @@ TEST_SUBMODULE(scoped_critical_section, m) {
     auto BoolWrapperHandle = py::handle(BoolWrapperClass);
     (void) BoolWrapperHandle.ptr(); // suppress unused variable warning
 
+    m.attr("has_barrier") =
 #ifdef PYBIND11_HAS_BARRIER
-    m.attr("has_barrier") = true;
+        true;
+#else
+        false;
+#endif
 
     m.def("test_scoped_critical_section",
           [BoolWrapperHandle]() -> void { test_scoped_critical_section(BoolWrapperHandle); });
@@ -200,7 +267,4 @@ TEST_SUBMODULE(scoped_critical_section, m) {
     m.def("test_scoped_critical_section2_same_object_no_deadlock", [BoolWrapperHandle]() -> void {
         test_scoped_critical_section2_same_object_no_deadlock(BoolWrapperHandle);
     });
-#else
-    m.attr("has_barrier") = false;
-#endif
 }

From 1b76b0b7072a8e4915a6e1d586dab345c7498e74 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Wed, 4 Jun 2025 15:30:57 +0800
Subject: [PATCH 18/19] test: add a summary comment for tests

---
 tests/test_scoped_critical_section.cpp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/test_scoped_critical_section.cpp b/tests/test_scoped_critical_section.cpp
index 7833e9c885..dc9a69e039 100644
--- a/tests/test_scoped_critical_section.cpp
+++ b/tests/test_scoped_critical_section.cpp
@@ -41,6 +41,11 @@ class BoolWrapper {
 
 #if defined(PYBIND11_HAS_BARRIER)
 
+// Modifying the C/C++ members of a Python object from multiple threads requires a critical section
+// to ensure thread safety and data integrity.
+// These tests use a scoped critical section to ensure that the Python object is accessed in a
+// thread-safe manner.
+
 void test_scoped_critical_section(const py::handle &cls) {
     auto barrier = std::barrier(2);
     auto bool_wrapper = cls(false);
@@ -113,7 +118,7 @@ void test_scoped_critical_section2(const py::handle &cls) {
             // Enter the critical section with two different objects.
             // This will ensure that the critical section is locked for both objects.
             py::scoped_critical_section lock{bool_wrapper1, bool_wrapper2};
-            // At this point, objects is locked by this thread via the scoped_critical_section.
+            // At this point, objects are locked by this thread via the scoped_critical_section.
             // This barrier will ensure that other threads wait until this thread has released
             // the critical section before proceeding.
             barrier.arrive_and_wait();

From 4b2410916b9991ff2c6344cf91a3dca71d91ce5a Mon Sep 17 00:00:00 2001
From: Henry Schreiner <henryschreineriii@gmail.com>
Date: Wed, 4 Jun 2025 10:59:07 -0400
Subject: [PATCH 19/19] refactor: simpler impl

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
---
 include/pybind11/critical_section.h | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/include/pybind11/critical_section.h b/include/pybind11/critical_section.h
index dd5bac9355..2d26412047 100644
--- a/include/pybind11/critical_section.h
+++ b/include/pybind11/critical_section.h
@@ -13,7 +13,7 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 class scoped_critical_section {
 public:
 #ifdef Py_GIL_DISABLED
-    scoped_critical_section(handle obj1, handle obj2) {
+    explicit scoped_critical_section(handle obj1, handle obj2 = handle{}) {
         if (obj1) {
             if (obj2) {
                 PyCriticalSection2_Begin(&section2, obj1.ptr(), obj2.ptr());
@@ -28,13 +28,6 @@ class scoped_critical_section {
         }
     }
 
-    explicit scoped_critical_section(handle obj) {
-        if (obj) {
-            PyCriticalSection_Begin(&section, obj.ptr());
-            rank = 1;
-        }
-    }
-
     ~scoped_critical_section() {
         if (rank == 1) {
             PyCriticalSection_End(&section);
@@ -43,8 +36,7 @@ class scoped_critical_section {
         }
     }
 #else
-    explicit scoped_critical_section(handle) {};
-    scoped_critical_section(handle, handle) {};
+    explicit scoped_critical_section(handle, handle = handle{}) {};
     ~scoped_critical_section() = default;
 #endif