From 7332300e39ca69edcecd6bd80cc1dd1a4c203192 Mon Sep 17 00:00:00 2001 From: Wenzel Jakob Date: Mon, 20 Mar 2017 21:54:49 +0100 Subject: [PATCH 1/7] nicer py::capsule destructor mechanism --- include/pybind11/eigen.h | 2 +- include/pybind11/pybind11.h | 4 ++-- include/pybind11/pytypes.h | 23 ++++++++++++++++++++--- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h index e9c44e5658..6abe8c48fb 100644 --- a/include/pybind11/eigen.h +++ b/include/pybind11/eigen.h @@ -235,7 +235,7 @@ handle eigen_ref_array(Type &src, handle parent = none()) { // not the Type of the pointer given is const. template ::value>> handle eigen_encapsulate(Type *src) { - capsule base(src, [](PyObject *o) { delete static_cast(PyCapsule_GetPointer(o, nullptr)); }); + capsule base(src, [](void *o) { delete static_cast(o); }); return eigen_ref_array(*src, base); } diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 42a19acb98..5976a36d81 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -294,8 +294,8 @@ class cpp_function : public function { rec->def->ml_meth = reinterpret_cast(*dispatcher); rec->def->ml_flags = METH_VARARGS | METH_KEYWORDS; - capsule rec_capsule(rec, [](PyObject *o) { - destruct((detail::function_record *) PyCapsule_GetPointer(o, nullptr)); + capsule rec_capsule(rec, [](void *ptr) { + destruct((detail::function_record *) ptr); }); object scope_module; diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index e8780912c8..ae54ef8bc9 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1004,10 +1004,27 @@ class capsule : public object { PYBIND11_OBJECT_DEFAULT(capsule, object, PyCapsule_CheckExact) PYBIND11_DEPRECATED("Use reinterpret_borrow() or reinterpret_steal()") capsule(PyObject *ptr, bool is_borrowed) : object(is_borrowed ? object(ptr, borrowed) : object(ptr, stolen)) { } - explicit capsule(const void *value, void (*destruct)(PyObject *) = nullptr) - : object(PyCapsule_New(const_cast(value), nullptr, destruct), stolen) { - if (!m_ptr) pybind11_fail("Could not allocate capsule object!"); + + explicit capsule(const void *value) + : object(PyCapsule_New(const_cast(value), nullptr, nullptr), stolen) { + if (!m_ptr) + pybind11_fail("Could not allocate capsule object!"); + } + + explicit capsule(const void *value, void (*destructor)(void *)) { + m_ptr = PyCapsule_New(const_cast(value), nullptr, [](PyObject *o) { + auto destructor = reinterpret_cast(PyCapsule_GetContext(o)); + void *ptr = PyCapsule_GetPointer(o, nullptr); + destructor(ptr); + }); + + if (!m_ptr) + pybind11_fail("Could not allocate capsule object!"); + + if (PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) + pybind11_fail("Could not set capsule context!"); } + template operator T *() const { T * result = static_cast(PyCapsule_GetPointer(m_ptr, nullptr)); if (!result) pybind11_fail("Unable to extract capsule contents!"); From af46e110d7e2902674d43f43b596a8ef2fa850f0 Mon Sep 17 00:00:00 2001 From: Wenzel Jakob Date: Mon, 20 Mar 2017 22:48:23 +0100 Subject: [PATCH 2/7] incorporated feedback by @dean0x7d --- include/pybind11/pytypes.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index ae54ef8bc9..da72a9f360 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1011,7 +1011,14 @@ class capsule : public object { pybind11_fail("Could not allocate capsule object!"); } - explicit capsule(const void *value, void (*destructor)(void *)) { + PYBIND11_DEPRECATED("Please pass a destructor that takes a void pointer as input") + capsule(const void *value, void (*destruct)(PyObject *)) + : object(PyCapsule_New(const_cast(value), nullptr, destruct), stolen) { + if (!m_ptr) + pybind11_fail("Could not allocate capsule object!"); + } + + capsule(const void *value, void (*destructor)(void *)) { m_ptr = PyCapsule_New(const_cast(value), nullptr, [](PyObject *o) { auto destructor = reinterpret_cast(PyCapsule_GetContext(o)); void *ptr = PyCapsule_GetPointer(o, nullptr); From 00e864de42aa8a83ebf4a4553db01425421d446d Mon Sep 17 00:00:00 2001 From: Wenzel Jakob Date: Wed, 22 Mar 2017 20:00:59 +0100 Subject: [PATCH 3/7] added destructor-only version of capsule & tests --- include/pybind11/pytypes.h | 10 ++++++++++ tests/test_python_types.cpp | 18 ++++++++++++++++++ tests/test_python_types.py | 19 +++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index da72a9f360..900c57564e 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1032,6 +1032,16 @@ class capsule : public object { pybind11_fail("Could not set capsule context!"); } + capsule(void (*destructor)()) { + m_ptr = PyCapsule_New(reinterpret_cast(destructor), nullptr, [](PyObject *o) { + auto destructor = reinterpret_cast(PyCapsule_GetPointer(o, nullptr)); + destructor(); + }); + + if (!m_ptr) + pybind11_fail("Could not allocate capsule object!"); + } + template operator T *() const { T * result = static_cast(PyCapsule_GetPointer(m_ptr, nullptr)); if (!result) pybind11_fail("Unable to extract capsule contents!"); diff --git a/tests/test_python_types.cpp b/tests/test_python_types.cpp index 399129fadf..5696239b4c 100644 --- a/tests/test_python_types.cpp +++ b/tests/test_python_types.cpp @@ -470,6 +470,24 @@ test_initializer python_types([](py::module &m) { m.def("return_none_bool", []() -> bool * { return nullptr; }); m.def("return_none_int", []() -> int * { return nullptr; }); m.def("return_none_float", []() -> float * { return nullptr; }); + + m.def("return_capsule_with_destructor", + []() { + py::print("creating capsule"); + return py::capsule([]() { + py::print("destructing capsule"); + }); + } + ); + + m.def("return_capsule_with_destructor_2", + []() { + py::print("creating capsule"); + return py::capsule((void *) 1234, [](void *ptr) { + py::print("destructing capsule: {}"_s.format((size_t) ptr)); + }); + } + ); }); #if defined(_MSC_VER) diff --git a/tests/test_python_types.py b/tests/test_python_types.py index 7ca020e894..73a6922006 100644 --- a/tests/test_python_types.py +++ b/tests/test_python_types.py @@ -512,3 +512,22 @@ def test_builtins_cast_return_none(): assert m.return_none_bool() is None assert m.return_none_int() is None assert m.return_none_float() is None + +def test_capsule_with_destructor(capture): + import pybind11_tests as m + with capture: + a = m.return_capsule_with_destructor() + del a + assert capture.unordered == """ + creating capsule + destructing capsule + """ + + with capture: + a = m.return_capsule_with_destructor_2() + del a + print(capture) + assert capture.unordered == """ + creating capsule + destructing capsule: 1234 + """ From b63f1b1010ce65f672662604035bbec70ffc84a8 Mon Sep 17 00:00:00 2001 From: Wenzel Jakob Date: Wed, 22 Mar 2017 20:14:00 +0100 Subject: [PATCH 4/7] added documentation for module destructors (fixes #733) --- docs/advanced/misc.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index 163fb0ee97..d98466512b 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -170,6 +170,20 @@ would be then able to access the data behind the same pointer. .. [#f6] https://docs.python.org/3/extending/extending.html#using-capsules +Module Destructors +================== + +pybind11 does not provide an explicit mechanism to invoke cleanup code at +module destruction time. In rare cases where such functionality is required, it +is possible to emulate it using Python capsules with a destruction callback. + +.. code-block:: cpp + + auto cleanup_callback = []() { + // perform cleanup here -- this function is called with the GIL held + }; + + m.add_object("_cleanup", py::capsule(cleanup_callback)); Generating documentation using Sphinx ===================================== From 601f4aa5e422a678ed95f9dc8ad5ea3e86294619 Mon Sep 17 00:00:00 2001 From: Wenzel Jakob Date: Wed, 22 Mar 2017 20:27:12 +0100 Subject: [PATCH 5/7] style fix --- tests/test_python_types.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_python_types.py b/tests/test_python_types.py index 73a6922006..3433608395 100644 --- a/tests/test_python_types.py +++ b/tests/test_python_types.py @@ -513,6 +513,7 @@ def test_builtins_cast_return_none(): assert m.return_none_int() is None assert m.return_none_float() is None + def test_capsule_with_destructor(capture): import pybind11_tests as m with capture: From 58178864407c84ca889ad9cc9c6a5c25f05ddeb3 Mon Sep 17 00:00:00 2001 From: Wenzel Jakob Date: Wed, 22 Mar 2017 21:46:01 +0100 Subject: [PATCH 6/7] added gc_collect calls --- tests/test_python_types.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_python_types.py b/tests/test_python_types.py index 3433608395..cca2263d36 100644 --- a/tests/test_python_types.py +++ b/tests/test_python_types.py @@ -519,6 +519,7 @@ def test_capsule_with_destructor(capture): with capture: a = m.return_capsule_with_destructor() del a + pytest.gc_collect() assert capture.unordered == """ creating capsule destructing capsule @@ -527,6 +528,7 @@ def test_capsule_with_destructor(capture): with capture: a = m.return_capsule_with_destructor_2() del a + pytest.gc_collect() print(capture) assert capture.unordered == """ creating capsule From 7f6262d554ccedf0d40cc3f4d496f948baf15966 Mon Sep 17 00:00:00 2001 From: Wenzel Jakob Date: Wed, 22 Mar 2017 21:48:35 +0100 Subject: [PATCH 7/7] cleanup leftover from debugging --- tests/test_python_types.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_python_types.py b/tests/test_python_types.py index cca2263d36..7956c7cc66 100644 --- a/tests/test_python_types.py +++ b/tests/test_python_types.py @@ -529,7 +529,6 @@ def test_capsule_with_destructor(capture): a = m.return_capsule_with_destructor_2() del a pytest.gc_collect() - print(capture) assert capture.unordered == """ creating capsule destructing capsule: 1234