diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 89f9cbd9b..990c21304 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -175,7 +175,7 @@ protected: #endif // UB without std::launder, but without breaking ABI and/or // a significant refactoring it's "impossible" to solve. - if (!std::is_trivially_destructible::value) + if (!std::is_trivially_destructible::value) rec->free_data = [](function_record *r) { auto data = PYBIND11_STD_LAUNDER((capture *) &r->data); (void) data; diff --git a/tests/test_callbacks.cpp b/tests/test_callbacks.cpp index a50771038..58688b6e8 100644 --- a/tests/test_callbacks.cpp +++ b/tests/test_callbacks.cpp @@ -81,16 +81,55 @@ TEST_SUBMODULE(callbacks, m) { }; // Export the payload constructor statistics for testing purposes: m.def("payload_cstats", &ConstructorStats::get); - /* Test cleanup of lambda closure */ - m.def("test_cleanup", []() -> std::function { + m.def("test_lambda_closure_cleanup", []() -> std::function { Payload p; + // In this situation, `Func` in the implementation of + // `cpp_function::initialize` is NOT trivially destructible. return [p]() { /* p should be cleaned up when the returned function is garbage collected */ (void) p; }; }); + class CppCallable { + public: + CppCallable() { track_default_created(this); } + ~CppCallable() { track_destroyed(this); } + CppCallable(const CppCallable &) { track_copy_created(this); } + CppCallable(CppCallable &&) noexcept { track_move_created(this); } + void operator()() {} + }; + + m.def("test_cpp_callable_cleanup", []() { + // Related issue: https://github.com/pybind/pybind11/issues/3228 + // Related PR: https://github.com/pybind/pybind11/pull/3229 + py::list alive_counts; + ConstructorStats &stat = ConstructorStats::get(); + alive_counts.append(stat.alive()); + { + CppCallable cpp_callable; + alive_counts.append(stat.alive()); + { + // In this situation, `Func` in the implementation of + // `cpp_function::initialize` IS trivially destructible, + // only `capture` is not. + py::cpp_function py_func(cpp_callable); + py::detail::silence_unused_warnings(py_func); + alive_counts.append(stat.alive()); + } + alive_counts.append(stat.alive()); + { + py::cpp_function py_func(std::move(cpp_callable)); + py::detail::silence_unused_warnings(py_func); + alive_counts.append(stat.alive()); + } + alive_counts.append(stat.alive()); + } + alive_counts.append(stat.alive()); + return alive_counts; + }); + // test_cpp_function_roundtrip /* Test if passing a function pointer from C++ -> Python -> C++ yields the original pointer */ m.def("dummy_function", &dummy_function); diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py index 9dc272a2d..edbb1890c 100644 --- a/tests/test_callbacks.py +++ b/tests/test_callbacks.py @@ -79,13 +79,18 @@ def test_keyword_args_and_generalized_unpacking(): def test_lambda_closure_cleanup(): - m.test_cleanup() + m.test_lambda_closure_cleanup() cstats = m.payload_cstats() assert cstats.alive() == 0 assert cstats.copy_constructions == 1 assert cstats.move_constructions >= 1 +def test_cpp_callable_cleanup(): + alive_counts = m.test_cpp_callable_cleanup() + assert alive_counts == [0, 1, 2, 1, 2, 1, 0] + + def test_cpp_function_roundtrip(): """Test if passing a function pointer from C++ -> Python -> C++ yields the original pointer"""