diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index dce875a6b9..77180ee9ae 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -124,10 +124,11 @@ struct argument_record { const char *descr; ///< Human-readable version of the argument value handle value; ///< Associated Python object bool convert : 1; ///< True if the argument is allowed to convert when loading + bool disown : 1; ///< TODO docs bool none : 1; ///< True if None is allowed when loading - argument_record(const char *name, const char *descr, handle value, bool convert, bool none) - : name(name), descr(descr), value(value), convert(convert), none(none) { } + argument_record(const char *name, const char *descr, handle value, bool convert, bool disown, bool none) + : name(name), descr(descr), value(value), convert(convert), disown(disown), none(none) { } }; /// Internal data structure which holds metadata about a bound function (signature, overloads, etc.) @@ -353,8 +354,8 @@ template <> struct process_attribute : process_attribu template <> struct process_attribute : process_attribute_default { static void init(const arg &a, function_record *r) { if (r->is_method && r->args.empty()) - r->args.emplace_back("self", nullptr, handle(), true /*convert*/, false /*none not allowed*/); - r->args.emplace_back(a.name, nullptr, handle(), !a.flag_noconvert, a.flag_none); + r->args.emplace_back("self", nullptr, handle(), true /*convert*/, false /*disown*/, false /*none not allowed*/); + r->args.emplace_back(a.name, nullptr, handle(), !a.flag_noconvert, a.flag_disown, a.flag_none); } }; @@ -362,7 +363,7 @@ template <> struct process_attribute : process_attribute_default { template <> struct process_attribute : process_attribute_default { static void init(const arg_v &a, function_record *r) { if (r->is_method && r->args.empty()) - r->args.emplace_back("self", nullptr /*descr*/, handle() /*parent*/, true /*convert*/, false /*none not allowed*/); + r->args.emplace_back("self", nullptr /*descr*/, handle() /*parent*/, true /*convert*/, false /*disown*/, false /*none not allowed*/); if (!a.value) { #if !defined(NDEBUG) @@ -385,7 +386,7 @@ template <> struct process_attribute : process_attribute_default { "Compile in debug mode for more information."); #endif } - r->args.emplace_back(a.name, a.descr, a.value.inc_ref(), !a.flag_noconvert, a.flag_none); + r->args.emplace_back(a.name, a.descr, a.value.inc_ref(), !a.flag_noconvert, a.flag_disown, a.flag_none); } }; diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 0aa632601e..664dacd213 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1371,6 +1371,7 @@ template class type_caster> template struct holder_helper { static auto get(const T &p) -> decltype(p.get()) { return p.get(); } + static void release(T &p) { p.release(); } }; /// Type caster for holder types like std::shared_ptr, etc. @@ -1714,16 +1715,19 @@ template arg_v operator=(T &&value) const; /// Indicate that the type should not be converted in the type caster arg &noconvert(bool flag = true) { flag_noconvert = flag; return *this; } + /// TODO: docs + arg &disown(bool flag = true) { flag_disown = flag; return *this; } /// Indicates that the argument should/shouldn't allow None (e.g. for nullable pointer args) arg &none(bool flag = true) { flag_none = flag; return *this; } const char *name; ///< If non-null, this is a named kwargs argument bool flag_noconvert : 1; ///< If set, do not allow conversion (requires a supporting type caster!) + bool flag_disown : 1; ///< TODO: docs bool flag_none : 1; ///< If set (the default), allow None to be passed to this argument }; @@ -1757,6 +1761,9 @@ struct arg_v : arg { /// Same as `arg::noconvert()`, but returns *this as arg_v&, not arg& arg_v &noconvert(bool flag = true) { arg::noconvert(flag); return *this; } + /// Same as `arg::disown()`, but returns *this as arg_v&, not arg& + arg_v &disown(bool flag = true) { arg::disown(flag); return *this; } + /// Same as `arg::nonone()`, but returns *this as arg_v&, not arg& arg_v &none(bool flag = true) { arg::none(flag); return *this; } diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 0383988eaf..786a9d71b2 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -613,6 +613,39 @@ class cpp_function : public function { call.args_convert.swap(second_pass_convert); } + + //printf("calling %s %s, args %d %lu\n", func.name, func.signature, func.nargs, func.args.size()); + for (size_t i = 0; i < func.args.size(); ++i) { + if (!func.args[i].disown) { + //printf("NOT disowning arg %lu\n", i); + continue; + } else { + //printf("DISOWNING arg %lu\n", i); + } + + auto &arg = call.args[i]; + if (!arg) + pybind11_fail("nothing to disown?!"); + + if (arg.is_none()) + continue; /* Nothing to disown */ + + auto inst = reinterpret_cast(arg.ptr()); + auto value_and_holder = values_and_holders(inst).begin(); + + + using holder_type = std::unique_ptr; // how do I get the proper one? + + auto &holder = value_and_holder->holder< holder_type >(); + + + holder_helper::release(holder); + + value_and_holder->set_holder_constructed(false); + inst->owned = false; + holder.~holder_type(); + } + // 6. Call the function. try { loader_life_support guard{}; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8f2f300ef7..c8df3df3b1 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -39,6 +39,7 @@ set(PYBIND11_TEST_FILES test_enum.cpp test_eval.cpp test_exceptions.cpp + test_disown.cpp test_factory_constructors.cpp test_iostream.cpp test_kwargs_and_defaults.cpp diff --git a/tests/test_disown.cpp b/tests/test_disown.cpp new file mode 100644 index 0000000000..cd5f102205 --- /dev/null +++ b/tests/test_disown.cpp @@ -0,0 +1,50 @@ +/* + tests/test_disown.cpp -- disown + + Copyright (c) 2016 Wenzel Jakob + Copyright (c) 2017 Attila Török + + All rights reserved. Use of this source code is governed by a + BSD-style license that can be found in the LICENSE file. +*/ + +#include "pybind11_tests.h" + +class Box { + int size; + static int num_boxes; + +public: + Box(int size): size(size) { py::print("Box created."); ++num_boxes; } + ~Box() { py::print("Box destroyed."); --num_boxes; } + + int get_size() { return size; } + static int get_num_boxes() { return num_boxes; } +}; + +int Box::num_boxes = 0; + +class Filter { + int threshold; + +public: + Filter(int threshold): threshold(threshold) { py::print("Filter created."); } + ~Filter() { py::print("Filter destroyed."); } + + void process(Box *box) { // ownership of box is taken + py::print("Box is processed by Filter."); + if (box->get_size() > threshold) + delete box; + // otherwise the box is leaked + }; +}; + +test_initializer disown([](py::module &m) { + py::class_(m, "Box") + .def(py::init()) + .def_static("get_num_boxes", &Box::get_num_boxes); + + py::class_(m, "Filter") + .def(py::init()) + .def("process", &Filter::process, py::arg("box").disown()); +}); diff --git a/tests/test_disown.py b/tests/test_disown.py new file mode 100644 index 0000000000..4ba5f52a31 --- /dev/null +++ b/tests/test_disown.py @@ -0,0 +1,42 @@ +import pytest + + +def test_disown_argument(capture): + from pybind11_tests import Box, Filter + + with capture: + filt = Filter(4) + assert capture == "Filter created." + with capture: + box_1 = Box(1) + box_8 = Box(8) + assert capture == """ + Box created. + Box created. + """ + + assert Box.get_num_boxes() == 2 + + with capture: + filt.process(box_1) # box_1 is not big enough, but process() leaks it + assert capture == "Box is processed by Filter." + + assert Box.get_num_boxes() == 2 + + with capture: + filt.process(box_8) # box_8 is destroyed by process() of filt + assert capture == """ + Box is processed by Filter. + Box destroyed. + """ + + assert Box.get_num_boxes() == 1 # box_1 still exists somehow, but we can't access it + + with capture: + del filt + del box_1 + del box_8 + pytest.gc_collect() + assert capture == "Filter destroyed." + + assert Box.get_num_boxes() == 1 # 1 box is leaked, and we can't do anything