From 59ef5307ae2a87f7f35f05fd0109d1a41d48c060 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 7 Oct 2022 12:24:49 -0700 Subject: [PATCH] [smart_holder] Add `gil_scoped_acquire` to `shared_ptr_trampoline_self_life_support` ctor. (#4196) * Add `gil_scoped_acquire` to `shared_ptr_trampoline_self_life_support` ctor. * Add test exercising fix & validation that the fix is needed (i.e. this is BROKEN). test_class_sh_trampoline_shared_ptr_cpp_arg.py::test_std_make_shared_factory[pass_through_shd_ptr] PASSED [ 87%] test_class_sh_trampoline_shared_ptr_cpp_arg.py::test_std_make_shared_factory[pass_through_shd_ptr_release_gil] FAILED [100%] ``` ================================================================= FAILURES ================================================================= ______________________________________ test_std_make_shared_factory[pass_through_shd_ptr_release_gil] ______________________________________ pass_through_func = @pytest.mark.parametrize( "pass_through_func", [m.pass_through_shd_ptr, m.pass_through_shd_ptr_release_gil] ) def test_std_make_shared_factory(pass_through_func): class PyChild(m.SpBase): def __init__(self): super().__init__(0) obj = PyChild() while True: > assert pass_through_func(obj) is obj E RuntimeError: NEEDED HERE: gil_scoped_acquire gil; ``` * Put back fix. --- include/pybind11/detail/smart_holder_type_casters.h | 1 + tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp | 3 +++ tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py | 7 +++++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index 52b37d591..5ff995003 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -400,6 +400,7 @@ struct shared_ptr_trampoline_self_life_support { PyObject *self; explicit shared_ptr_trampoline_self_life_support(instance *inst) : self{reinterpret_cast(inst)} { + gil_scoped_acquire gil; Py_INCREF(self); } void operator()(void *) { diff --git a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp b/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp index 5773f08e8..9e44927a8 100644 --- a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp +++ b/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp @@ -69,6 +69,9 @@ TEST_SUBMODULE(class_sh_trampoline_shared_ptr_cpp_arg, m) { .def("has_python_instance", &SpBase::has_python_instance); m.def("pass_through_shd_ptr", pass_through_shd_ptr); + m.def("pass_through_shd_ptr_release_gil", + pass_through_shd_ptr, + py::call_guard()); // PR #4196 py::classh(m, "SpBaseTester") .def(py::init<>()) diff --git a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py b/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py index 912d22cbe..b9cde4f2c 100644 --- a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py +++ b/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py @@ -132,12 +132,15 @@ def test_infinite(): break # Comment out for manual leak checking (use `top` command). -def test_std_make_shared_factory(): +@pytest.mark.parametrize( + "pass_through_func", [m.pass_through_shd_ptr, m.pass_through_shd_ptr_release_gil] +) +def test_std_make_shared_factory(pass_through_func): class PyChild(m.SpBase): def __init__(self): super().__init__(0) obj = PyChild() while True: - assert m.pass_through_shd_ptr(obj) is obj + assert pass_through_func(obj) is obj break # Comment out for manual leak checking (use `top` command).