diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 07c0943006..05d6755896 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -103,6 +103,10 @@ # define PYBIND11_DTOR_CONSTEXPR #endif +#if defined(PYBIND11_CPP20) && defined(__has_include) && __has_include() +# define PYBIND11_HAS_STD_BARRIER 1 +#endif + // Compiler version assertions #if defined(__INTEL_COMPILER) # if __INTEL_COMPILER < 1800 diff --git a/include/pybind11/gil.h b/include/pybind11/gil.h index 4222a035f4..9e799b3cf7 100644 --- a/include/pybind11/gil.h +++ b/include/pybind11/gil.h @@ -120,7 +120,11 @@ class gil_scoped_acquire { pybind11_fail("scoped_acquire::dec_ref(): internal error!"); } # endif + // Make sure that PyThreadState_Clear is not recursively called by finalizers. + // See issue #5827 + ++tstate->gilstate_counter; PyThreadState_Clear(tstate); + --tstate->gilstate_counter; if (active) { PyThreadState_DeleteCurrent(); } diff --git a/tests/test_scoped_critical_section.cpp b/tests/test_scoped_critical_section.cpp index dc9a69e039..7401eb09f8 100644 --- a/tests/test_scoped_critical_section.cpp +++ b/tests/test_scoped_critical_section.cpp @@ -7,8 +7,7 @@ #include #include -#if defined(PYBIND11_CPP20) && defined(__has_include) && __has_include() -# define PYBIND11_HAS_BARRIER 1 +#if defined(PYBIND11_HAS_STD_BARRIER) # include #endif @@ -39,7 +38,7 @@ class BoolWrapper { std::atomic_bool value_{false}; }; -#if defined(PYBIND11_HAS_BARRIER) +#if defined(PYBIND11_HAS_STD_BARRIER) // Modifying the C/C++ members of a Python object from multiple threads requires a critical section // to ensure thread safety and data integrity. @@ -259,7 +258,7 @@ TEST_SUBMODULE(scoped_critical_section, m) { (void) BoolWrapperHandle.ptr(); // suppress unused variable warning m.attr("has_barrier") = -#ifdef PYBIND11_HAS_BARRIER +#ifdef PYBIND11_HAS_STD_BARRIER true; #else false; diff --git a/tests/test_thread.cpp b/tests/test_thread.cpp index eabf39afa1..131bd87710 100644 --- a/tests/test_thread.cpp +++ b/tests/test_thread.cpp @@ -15,6 +15,10 @@ #include #include +#if defined(PYBIND11_HAS_STD_BARRIER) +# include +#endif + namespace py = pybind11; namespace { @@ -34,7 +38,6 @@ EmptyStruct SharedInstance; } // namespace TEST_SUBMODULE(thread, m) { - py::class_(m, "IntStruct").def(py::init([](const int i) { return IntStruct(i); })); // implicitly_convertible uses loader_life_support when an implicit @@ -67,6 +70,39 @@ TEST_SUBMODULE(thread, m) { py::class_(m, "EmptyStruct") .def_readonly_static("SharedInstance", &SharedInstance); +#if defined(PYBIND11_HAS_STD_BARRIER) + // In the free-threaded build, during PyThreadState_Clear, removing the thread from the biased + // reference counting table may call destructors. Make sure that it doesn't crash. + m.def("test_pythread_state_clear_destructor", [](py::type cls) { + py::handle obj; + + std::barrier barrier{2}; + std::thread thread1{[&]() { + py::gil_scoped_acquire gil; + obj = cls().release(); + barrier.arrive_and_wait(); + }}; + std::thread thread2{[&]() { + py::gil_scoped_acquire gil; + barrier.arrive_and_wait(); + // ob_ref_shared becomes negative; transition to the queued state + obj.dec_ref(); + }}; + + // jthread is not supported by Apple Clang + thread1.join(); + thread2.join(); + }); +#endif + + m.attr("defined_PYBIND11_HAS_STD_BARRIER") = +#ifdef PYBIND11_HAS_STD_BARRIER + true; +#else + false; +#endif + m.def("acquire_gil", []() { py::gil_scoped_acquire gil_acquired; }); + // NOTE: std::string_view also uses loader_life_support to ensure that // the string contents remain alive, but that's a C++ 17 feature. } diff --git a/tests/test_thread.py b/tests/test_thread.py index e9d7bafb2f..d302c382c2 100644 --- a/tests/test_thread.py +++ b/tests/test_thread.py @@ -5,6 +5,7 @@ import pytest +import env from pybind11_tests import thread as m @@ -66,3 +67,14 @@ def access_shared_instance(): thread.start() for thread in threads: thread.join() + + +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") +@pytest.mark.skipif(not m.defined_PYBIND11_HAS_STD_BARRIER, reason="no ") +@pytest.mark.skipif(env.sys_is_gil_enabled(), reason="Deadlock with the GIL") +def test_pythread_state_clear_destructor(): + class Foo: + def __del__(self): + m.acquire_gil() + + m.test_pythread_state_clear_destructor(Foo)