From fc1a278da31c9f05f07337c3496e0123d77e494b Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 1 Apr 2023 12:41:57 -0700 Subject: [PATCH 1/8] Snapshot of pybind/pybind11#4601 (squashed). --- include/pybind11/cast.h | 12 +++- include/pybind11/detail/common.h | 4 ++ include/pybind11/type_caster_pyobject_ptr.h | 57 +++++++++++++++++ tests/CMakeLists.txt | 1 + tests/test_type_caster_pyobject_ptr.cpp | 52 +++++++++++++++ tests/test_type_caster_pyobject_ptr.py | 71 +++++++++++++++++++++ 6 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 include/pybind11/type_caster_pyobject_ptr.h create mode 100644 tests/test_type_caster_pyobject_ptr.cpp create mode 100644 tests/test_type_caster_pyobject_ptr.py diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index f40142b8..72f50d2e 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1207,7 +1207,11 @@ make_caster load_type(const handle &handle) { PYBIND11_NAMESPACE_END(detail) // pytype -> C++ type -template ::value, int> = 0> +template ::value + && !detail::is_same_ignoring_cvref::value, + int> + = 0> T cast(const handle &handle) { using namespace detail; constexpr bool is_enum_cast = type_uses_type_caster_enum_type>::value; @@ -1231,6 +1235,12 @@ T cast(const handle &handle) { return T(reinterpret_borrow(handle)); } +template ::value, int> = 0> +T cast(const handle &handle) { + return handle.ptr(); +} + // C++ type -> py::object template ::value, int> = 0> object cast(T &&value, diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index def3b133..8b211fd5 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -751,6 +751,10 @@ template using remove_cvref_t = typename remove_cvref::type; #endif +/// Example usage: is_same_ignoring_cvref +template +using is_same_ignoring_cvref = std::is_same, U>; + /// Index sequences #if defined(PYBIND11_CPP14) using std::index_sequence; diff --git a/include/pybind11/type_caster_pyobject_ptr.h b/include/pybind11/type_caster_pyobject_ptr.h new file mode 100644 index 00000000..5d538d20 --- /dev/null +++ b/include/pybind11/type_caster_pyobject_ptr.h @@ -0,0 +1,57 @@ +// Copyright (c) 2023 The pybind Community. + +#pragma once + +#include "detail/common.h" +#include "detail/descr.h" +#include "cast.h" +#include "pytypes.h" + +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) +PYBIND11_NAMESPACE_BEGIN(detail) + +template <> +class type_caster { +public: + static constexpr auto name = const_name("PyObject *"); + + // This overload is purely to guard against accidents. + template ::value, int> = 0> + static handle cast(T &&, return_value_policy, handle /*parent*/) { + static_assert(is_same_ignoring_cvref::value, + "Invalid C++ type T for to-Python conversion (type_caster)."); + return nullptr; // Unreachable. + } + + static handle cast(PyObject *src, return_value_policy policy, handle /*parent*/) { + if (src == nullptr) { + throw error_already_set(); + } + if (PyErr_Occurred()) { + raise_from(PyExc_SystemError, "src != nullptr but PyErr_Occurred()"); + throw error_already_set(); + } + if (policy == return_value_policy::take_ownership) { + return src; + } + Py_INCREF(src); + return src; + } + + bool load(handle src, bool) { + value = reinterpret_borrow(src); + return true; + } + + template + using cast_op_type = PyObject *; + + explicit operator PyObject *() { return value.ptr(); } + +private: + object value; +}; + +PYBIND11_NAMESPACE_END(detail) +PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 269da163..58ea9aa3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -180,6 +180,7 @@ set(PYBIND11_TEST_FILES test_thread test_type_caster_odr_guard_1 test_type_caster_odr_guard_2 + test_type_caster_pyobject_ptr test_union test_virtual_functions) diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp new file mode 100644 index 00000000..71fbd383 --- /dev/null +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -0,0 +1,52 @@ +#include +#include + +#include "pybind11_tests.h" + +TEST_SUBMODULE(type_caster_pyobject_ptr, m) { + m.def("cast_from_pyobject_ptr", []() { + PyObject *ptr = PyLong_FromLongLong(6758L); + py::object retval = py::cast(ptr, py::return_value_policy::take_ownership); + return retval; + }); + m.def("cast_to_pyobject_ptr", [](py::handle obj) { + auto *ptr = py::cast(obj); + return bool(PyTuple_CheckExact(ptr)); + }); + + m.def( + "return_pyobject_ptr", + []() { return PyLong_FromLongLong(2314L); }, + py::return_value_policy::take_ownership); + m.def("pass_pyobject_ptr", [](PyObject *obj) { return bool(PyTuple_CheckExact(obj)); }); + + m.def("call_callback_with_object_return", + [](const std::function &cb, int mode) { return cb(mode); }); + m.def("call_callback_with_handle_return", + [](const std::function &cb, int mode) { return cb(mode); }); + // + m.def("call_callback_with_pyobject_ptr_return", + [](const std::function &cb, int mode) { return cb(mode); }); + m.def("call_callback_with_pyobject_ptr_arg", + [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }); + + m.def("cast_to_pyobject_ptr_nullptr", [](bool set_error) { + if (set_error) { + PyErr_SetString(PyExc_RuntimeError, "Reflective of healthy error handling."); + } + PyObject *ptr = nullptr; + py::cast(ptr); + }); + + m.def("cast_to_pyobject_ptr_non_nullptr_with_error_set", []() { + PyErr_SetString(PyExc_RuntimeError, "Reflective of unhealthy error handling."); + py::cast(Py_None); + }); + +#ifdef PYBIND11_NO_COMPILE_SECTION // Change to ifndef for manual testing. + { + PyObject *ptr = nullptr; + (void) py::cast(*ptr); + } +#endif +} diff --git a/tests/test_type_caster_pyobject_ptr.py b/tests/test_type_caster_pyobject_ptr.py new file mode 100644 index 00000000..69934952 --- /dev/null +++ b/tests/test_type_caster_pyobject_ptr.py @@ -0,0 +1,71 @@ +import pytest + +from pybind11_tests import type_caster_pyobject_ptr as m + + +def test_cast_from_pyobject_ptr(): + assert m.cast_from_pyobject_ptr() == 6758 + + +def test_cast_to_pyobject_ptr(): + assert m.cast_to_pyobject_ptr(()) + assert not m.cast_to_pyobject_ptr({}) + + +def test_return_pyobject_ptr(): + assert m.return_pyobject_ptr() == 2314 + + +def test_pass_pyobject_ptr(): + assert m.pass_pyobject_ptr(()) + assert not m.pass_pyobject_ptr({}) + + +@pytest.mark.parametrize( + "call_callback", + [ + m.call_callback_with_object_return, + m.call_callback_with_handle_return, + m.call_callback_with_pyobject_ptr_return, + ], +) +def test_call_callback_with_object_return(call_callback): + def cb(mode): + if mode == 0: + return 10 + if mode == 1: + return "One" + raise NotImplementedError(f"Unknown mode: {mode}") + + assert call_callback(cb, 0) == 10 + assert call_callback(cb, 1) == "One" + with pytest.raises(NotImplementedError, match="Unknown mode: 2"): + call_callback(cb, 2) + + +def test_call_callback_with_pyobject_ptr_arg(): + def cb(obj): + return isinstance(obj, tuple) + + assert m.call_callback_with_pyobject_ptr_arg(cb, ()) + assert not m.call_callback_with_pyobject_ptr_arg(cb, {}) + + +@pytest.mark.parametrize("set_error", [True, False]) +def test_cast_to_python_nullptr(set_error): + expected = { + True: r"^Reflective of healthy error handling\.$", + False: ( + r"^Internal error: pybind11::error_already_set called " + r"while Python error indicator not set\.$" + ), + }[set_error] + with pytest.raises(RuntimeError, match=expected): + m.cast_to_pyobject_ptr_nullptr(set_error) + + +def test_cast_to_python_non_nullptr_with_error_set(): + with pytest.raises(SystemError) as excinfo: + m.cast_to_pyobject_ptr_non_nullptr_with_error_set() + assert str(excinfo.value) == "src != nullptr but PyErr_Occurred()" + assert str(excinfo.value.__cause__) == "Reflective of unhealthy error handling." From 9a972b8caa19ea8b2bdf26ae5cb2c533e8924b5b Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 1 Apr 2023 12:55:42 -0700 Subject: [PATCH 2/8] Add new header file to CMakeLists.txt and tests/extra_python_package/test_files.py --- CMakeLists.txt | 3 ++- tests/extra_python_package/test_files.py | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c3f5207b..dc804b65 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -151,7 +151,8 @@ set(PYBIND11_HEADERS include/pybind11/stl.h include/pybind11/stl_bind.h include/pybind11/stl/filesystem.h - include/pybind11/trampoline_self_life_support.h) + include/pybind11/trampoline_self_life_support.h + include/pybind11/type_caster_pyobject_ptr.h) # Compare with grep and warn if mismatched if(PYBIND11_MASTER_PROJECT AND NOT CMAKE_VERSION VERSION_LESS 3.12) diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index 605743c1..4d12872d 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -46,6 +46,7 @@ "include/pybind11/stl.h", "include/pybind11/stl_bind.h", "include/pybind11/trampoline_self_life_support.h", + "include/pybind11/type_caster_pyobject_ptr.h", } detail_headers = { From a311d27384952d65ee01c09f2c022f672168fc22 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 2 Apr 2023 07:23:50 -0700 Subject: [PATCH 3/8] to-Pyton-cast: `return_value_policy::_clif_automatic` => take ownership --- include/pybind11/type_caster_pyobject_ptr.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/pybind11/type_caster_pyobject_ptr.h b/include/pybind11/type_caster_pyobject_ptr.h index 5d538d20..3befcfc6 100644 --- a/include/pybind11/type_caster_pyobject_ptr.h +++ b/include/pybind11/type_caster_pyobject_ptr.h @@ -32,7 +32,8 @@ class type_caster { raise_from(PyExc_SystemError, "src != nullptr but PyErr_Occurred()"); throw error_already_set(); } - if (policy == return_value_policy::take_ownership) { + if (policy == return_value_policy::take_ownership + || policy == return_value_policy::_clif_automatic) { return src; } Py_INCREF(src); From f002936adea8926f5363f344239ed6a9c3cfb5d7 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 3 Apr 2023 06:41:51 -0700 Subject: [PATCH 4/8] `handle.inc_ref()` in `T cast(const handle &handle)` specialization for `PyObject *` --- include/pybind11/cast.h | 2 +- tests/test_type_caster_pyobject_ptr.cpp | 7 ++----- tests/test_type_caster_pyobject_ptr.py | 27 ++++++++++++++----------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 72f50d2e..c7f68ca2 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1238,7 +1238,7 @@ T cast(const handle &handle) { template ::value, int> = 0> T cast(const handle &handle) { - return handle.ptr(); + return handle.inc_ref().ptr(); } // C++ type -> py::object diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index 71fbd383..3ec0bde1 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -21,12 +21,9 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { m.def("pass_pyobject_ptr", [](PyObject *obj) { return bool(PyTuple_CheckExact(obj)); }); m.def("call_callback_with_object_return", - [](const std::function &cb, int mode) { return cb(mode); }); - m.def("call_callback_with_handle_return", - [](const std::function &cb, int mode) { return cb(mode); }); - // + [](const std::function &cb, int value) { return cb(value); }); m.def("call_callback_with_pyobject_ptr_return", - [](const std::function &cb, int mode) { return cb(mode); }); + [](const std::function &cb, int value) { return cb(value); }); m.def("call_callback_with_pyobject_ptr_arg", [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }); diff --git a/tests/test_type_caster_pyobject_ptr.py b/tests/test_type_caster_pyobject_ptr.py index 69934952..6271e920 100644 --- a/tests/test_type_caster_pyobject_ptr.py +++ b/tests/test_type_caster_pyobject_ptr.py @@ -21,26 +21,29 @@ def test_pass_pyobject_ptr(): assert not m.pass_pyobject_ptr({}) +class ValueHolder: + def __init__(self, value): + self.value = value + + @pytest.mark.parametrize( "call_callback", [ m.call_callback_with_object_return, - m.call_callback_with_handle_return, m.call_callback_with_pyobject_ptr_return, ], ) def test_call_callback_with_object_return(call_callback): - def cb(mode): - if mode == 0: - return 10 - if mode == 1: - return "One" - raise NotImplementedError(f"Unknown mode: {mode}") - - assert call_callback(cb, 0) == 10 - assert call_callback(cb, 1) == "One" - with pytest.raises(NotImplementedError, match="Unknown mode: 2"): - call_callback(cb, 2) + def cb(value): + if value < 0: + raise ValueError("Raised from cb") + # Return a temporary user-defined object, to maximize sensitivity of this test. + return ValueHolder(1000 - value) + + assert call_callback(cb, 287).value == 713 + + with pytest.raises(ValueError, match="^Raised from cb$"): + call_callback(cb, -1) def test_call_callback_with_pyobject_ptr_arg(): From b3f3744c5006c7e04833446ff1b95f1c28bcc18c Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 3 Apr 2023 07:41:48 -0700 Subject: [PATCH 5/8] Guard against accidental leaking by requiring `return_value_policy::reference` or `automatic_reference` --- include/pybind11/type_caster_pyobject_ptr.h | 8 ++++++-- tests/test_type_caster_pyobject_ptr.cpp | 13 +++++++++---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/include/pybind11/type_caster_pyobject_ptr.h b/include/pybind11/type_caster_pyobject_ptr.h index 3befcfc6..77abae66 100644 --- a/include/pybind11/type_caster_pyobject_ptr.h +++ b/include/pybind11/type_caster_pyobject_ptr.h @@ -36,8 +36,12 @@ class type_caster { || policy == return_value_policy::_clif_automatic) { return src; } - Py_INCREF(src); - return src; + if (policy == return_value_policy::reference + || policy == return_value_policy::automatic_reference) { + return handle(src).inc_ref(); + } + pybind11_fail("type_caster::cast(): unsupported return_value_policy: " + + std::to_string(static_cast(policy))); } bool load(handle src, bool) { diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index 3ec0bde1..4e476d34 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -22,10 +22,15 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { m.def("call_callback_with_object_return", [](const std::function &cb, int value) { return cb(value); }); - m.def("call_callback_with_pyobject_ptr_return", - [](const std::function &cb, int value) { return cb(value); }); - m.def("call_callback_with_pyobject_ptr_arg", - [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }); + m.def( + "call_callback_with_pyobject_ptr_return", + [](const std::function &cb, int value) { return cb(value); }, + py::return_value_policy::take_ownership); + m.def( + "call_callback_with_pyobject_ptr_arg", + [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }, + py::arg("cb"), // This triggers return_value_policy::automatic_reference + py::arg("obj")); m.def("cast_to_pyobject_ptr_nullptr", [](bool set_error) { if (set_error) { From f4bb50081e7bbd53a4ec6e50189b03e73edcbb10 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 4 Apr 2023 16:33:00 -0700 Subject: [PATCH 6/8] Fix oversight in test (to resolve a valgrind leak detection error) and add a related comment in cast.h. No production code changes. Make tests more sensitive by using `ValueHolder` instead of empty tuples and dicts. Manual leak checks with `while True:` & top command repeated for all tests. --- include/pybind11/cast.h | 4 ++++ tests/test_type_caster_pyobject_ptr.cpp | 16 +++++++++++----- tests/test_type_caster_pyobject_ptr.py | 23 ++++++++++------------- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index c7f68ca2..a53745ae 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1235,6 +1235,10 @@ T cast(const handle &handle) { return T(reinterpret_borrow(handle)); } +// Note that `cast(obj)` increments the reference count of `obj`. +// This is necessary for the case that `obj` is a temporary. +// It is the responsibility of the caller to ensure that the reference count +// is decremented. template ::value, int> = 0> T cast(const handle &handle) { diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index 4e476d34..e5fd6b9a 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -6,19 +6,25 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { m.def("cast_from_pyobject_ptr", []() { PyObject *ptr = PyLong_FromLongLong(6758L); - py::object retval = py::cast(ptr, py::return_value_policy::take_ownership); - return retval; + return py::cast(ptr, py::return_value_policy::take_ownership); }); m.def("cast_to_pyobject_ptr", [](py::handle obj) { + auto rc1 = obj.ref_count(); auto *ptr = py::cast(obj); - return bool(PyTuple_CheckExact(ptr)); + auto rc2 = obj.ref_count(); + if (rc2 != rc1 + 1) { + return -1; + } + return 100 - py::reinterpret_steal(ptr).attr("value").cast(); }); m.def( "return_pyobject_ptr", []() { return PyLong_FromLongLong(2314L); }, py::return_value_policy::take_ownership); - m.def("pass_pyobject_ptr", [](PyObject *obj) { return bool(PyTuple_CheckExact(obj)); }); + m.def("pass_pyobject_ptr", [](PyObject *ptr) { + return 200 - py::reinterpret_borrow(ptr).attr("value").cast(); + }); m.def("call_callback_with_object_return", [](const std::function &cb, int value) { return cb(value); }); @@ -28,7 +34,7 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { py::return_value_policy::take_ownership); m.def( "call_callback_with_pyobject_ptr_arg", - [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }, + [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }, py::arg("cb"), // This triggers return_value_policy::automatic_reference py::arg("obj")); diff --git a/tests/test_type_caster_pyobject_ptr.py b/tests/test_type_caster_pyobject_ptr.py index 6271e920..53604954 100644 --- a/tests/test_type_caster_pyobject_ptr.py +++ b/tests/test_type_caster_pyobject_ptr.py @@ -3,13 +3,18 @@ from pybind11_tests import type_caster_pyobject_ptr as m +# For use as a temporary user-defined object, to maximize sensitivity of the tests below. +class ValueHolder: + def __init__(self, value): + self.value = value + + def test_cast_from_pyobject_ptr(): assert m.cast_from_pyobject_ptr() == 6758 def test_cast_to_pyobject_ptr(): - assert m.cast_to_pyobject_ptr(()) - assert not m.cast_to_pyobject_ptr({}) + assert m.cast_to_pyobject_ptr(ValueHolder(24)) == 76 def test_return_pyobject_ptr(): @@ -17,13 +22,7 @@ def test_return_pyobject_ptr(): def test_pass_pyobject_ptr(): - assert m.pass_pyobject_ptr(()) - assert not m.pass_pyobject_ptr({}) - - -class ValueHolder: - def __init__(self, value): - self.value = value + assert m.pass_pyobject_ptr(ValueHolder(82)) == 118 @pytest.mark.parametrize( @@ -37,7 +36,6 @@ def test_call_callback_with_object_return(call_callback): def cb(value): if value < 0: raise ValueError("Raised from cb") - # Return a temporary user-defined object, to maximize sensitivity of this test. return ValueHolder(1000 - value) assert call_callback(cb, 287).value == 713 @@ -48,10 +46,9 @@ def cb(value): def test_call_callback_with_pyobject_ptr_arg(): def cb(obj): - return isinstance(obj, tuple) + return 300 - obj.value - assert m.call_callback_with_pyobject_ptr_arg(cb, ()) - assert not m.call_callback_with_pyobject_ptr_arg(cb, {}) + assert m.call_callback_with_pyobject_ptr_arg(cb, ValueHolder(39)) == 261 @pytest.mark.parametrize("set_error", [True, False]) From a72ccc609576af48e8fb5b0318f9ece24a582542 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 4 Apr 2023 21:49:47 -0700 Subject: [PATCH 7/8] Add tests for interop with stl.h `list_caster` (No production code changes.) --- tests/test_type_caster_pyobject_ptr.cpp | 47 +++++++++++++++++++++++++ tests/test_type_caster_pyobject_ptr.py | 20 +++++++++++ 2 files changed, 67 insertions(+) diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index e5fd6b9a..cd65721e 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -1,8 +1,25 @@ #include +#include #include #include "pybind11_tests.h" +#include +#include + +namespace { + +std::vector make_vector_pyobject_ptr(const py::object &ValueHolder) { + std::vector vec_obj; + for (int i = 1; i < 3; i++) { + vec_obj.push_back(ValueHolder(i * 93).release().ptr()); + } + // This vector now owns the refcounts. + return vec_obj; +} + +} // namespace + TEST_SUBMODULE(type_caster_pyobject_ptr, m) { m.def("cast_from_pyobject_ptr", []() { PyObject *ptr = PyLong_FromLongLong(6758L); @@ -51,6 +68,36 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { py::cast(Py_None); }); + m.def("pass_list_pyobject_ptr", [](const std::vector &vec_obj) { + int acc = 0; + for (const auto &ptr : vec_obj) { + acc = acc * 1000 + py::reinterpret_borrow(ptr).attr("value").cast(); + } + return acc; + }); + + m.def("return_list_pyobject_ptr_take_ownership", + make_vector_pyobject_ptr, + // Ownership is transferred one-by-one when the vector is converted to a Python list. + py::return_value_policy::take_ownership); + + m.def("return_list_pyobject_ptr_reference", + make_vector_pyobject_ptr, + // Ownership is not transferred. + py::return_value_policy::reference); + + m.def("dec_ref_each_pyobject_ptr", [](const std::vector &vec_obj) { + std::size_t i = 0; + for (; i < vec_obj.size(); i++) { + py::handle h(vec_obj[i]); + if (static_cast(h.ref_count()) < i) { + break; + } + h.dec_ref(); + } + return i; + }); + #ifdef PYBIND11_NO_COMPILE_SECTION // Change to ifndef for manual testing. { PyObject *ptr = nullptr; diff --git a/tests/test_type_caster_pyobject_ptr.py b/tests/test_type_caster_pyobject_ptr.py index 53604954..fcb084a5 100644 --- a/tests/test_type_caster_pyobject_ptr.py +++ b/tests/test_type_caster_pyobject_ptr.py @@ -69,3 +69,23 @@ def test_cast_to_python_non_nullptr_with_error_set(): m.cast_to_pyobject_ptr_non_nullptr_with_error_set() assert str(excinfo.value) == "src != nullptr but PyErr_Occurred()" assert str(excinfo.value.__cause__) == "Reflective of unhealthy error handling." + + +def test_pass_list_pyobject_ptr(): + acc = m.pass_list_pyobject_ptr([ValueHolder(842), ValueHolder(452)]) + assert acc == 842452 + + +def test_return_list_pyobject_ptr_take_ownership(): + vec_obj = m.return_list_pyobject_ptr_take_ownership(ValueHolder) + assert [e.value for e in vec_obj] == [93, 186] + + +def test_return_list_pyobject_ptr_reference(): + vec_obj = m.return_list_pyobject_ptr_reference(ValueHolder) + assert [e.value for e in vec_obj] == [93, 186] + # Commenting out the next `assert` will leak the Python references. + # An easy way to see evidence of the leaks: + # Insert `while True:` as the first line of this function and monitor the + # process RES (Resident Memory Size) with the Unix top command. + assert m.dec_ref_each_pyobject_ptr(vec_obj) == 2 From 099c7a130172a82965b01d7adc3150361baae908 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 4 Apr 2023 22:30:30 -0700 Subject: [PATCH 8/8] Bug fix in test. Minor comment enhancements. --- include/pybind11/detail/common.h | 2 +- tests/test_type_caster_pyobject_ptr.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 8b211fd5..5374b5a2 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -751,7 +751,7 @@ template using remove_cvref_t = typename remove_cvref::type; #endif -/// Example usage: is_same_ignoring_cvref +/// Example usage: is_same_ignoring_cvref::value template using is_same_ignoring_cvref = std::is_same, U>; diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index cd65721e..6135ded4 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -90,8 +90,8 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { std::size_t i = 0; for (; i < vec_obj.size(); i++) { py::handle h(vec_obj[i]); - if (static_cast(h.ref_count()) < i) { - break; + if (static_cast(h.ref_count()) < 2) { + break; // Something is badly wrong. } h.dec_ref(); }