Zengtudor/pybind11
1
0
Fork 0
mirror of https://github.com/pybind/pybind11.git synced 2024-11-21 20:55:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: memory leak in cpp_function (#3228) (#3229)

Browse Source 
* fix: memory leak in cpp_function (#3228)

* add a test case to check objects are deconstructed in cpp_function

* update the test case about cpp_function

* fix the test case about cpp_function: remove "noexcept"

* Actually calling func. CHECK(stat.alive() == 2); Manually verified that the new tests fails without the change in pybind11.h

* Moving new test to test_callbacks.cpp,py, with small enhancements.

* Removing new test from test_interpreter.cpp (after it was moved to test_callbacks.cpp,py). This restores test_interpreter.cpp to the current state on master.

* Using py::detail::silence_unused_warnings(py_func); to make the intent clear.

Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
This commit is contained in:
Tailing Yuan 2021-08-31 09:48:33 +08:00 committed by GitHub
parent 76d939de53
commit d6474ed7d2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 48 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/pybind11/pybind11.h
View File

@ -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<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;

43
tests/test_callbacks.cpp
View File

@ -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);

7
tests/test_callbacks.py
View File

@ -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"""