fix: memory leak in cpp_function (#3228)

include/pybind11/pybind11.h

@@ -175,7 +175,7 @@ class cpp_function : public function {
 #endif
             // UB without std::launder, but without breaking ABI and/or
             // a significant refactoring it's "impossible" to solve.
-            if (!std::is_trivially_destructible<Func>::value)
+            if (!std::is_trivially_destructible<capture>::value)
                 rec->free_data = [](function_record *r) {
                     auto data = PYBIND11_STD_LAUNDER((capture *) &r->data);
                     (void) data;

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<Payload>);
-    /* Test cleanup of lambda closure */
-    m.def("test_cleanup", []() -> std::function<void()> {
+    m.def("test_lambda_closure_cleanup", []() -> std::function<void()> {
         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<CppCallable>();
+        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);

tests/test_callbacks.py

@@ -79,13 +79,18 @@ def f(*args, **kwargs):
 
 
 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"""