type_caster_generic: add set_foreign_holder method for subclasses to implement

.codespell-ignore-lines

@@ -2,6 +2,7 @@ template <op_id id, op_type ot, typename L = undefined_t, typename R = undefined
     template <typename ThisT>
         auto &this_ = static_cast<ThisT &>(*this);
                 if (load_impl<ThisT>(temp, false)) {
+                return load_impl<ThisT>(src, false);
         ssize_t nd = 0;
         auto trivial = broadcast(buffers, nd, shape);
         auto ndim = (size_t) nd;

include/pybind11/cast.h

@@ -863,6 +863,71 @@ struct holder_helper {
     static auto get(const T &p) -> decltype(p.get()) { return p.get(); }
 };
 
+struct holder_caster_foreign_helpers {
+    struct py_deleter {
+        void operator()(const void *) const noexcept {
+            // Don't run the deleter if the interpreter has been shut down
+            if (Py_IsInitialized() == 0) {
+                return;
+            }
+            gil_scoped_acquire guard;
+            Py_DECREF(o);
+        }
+
+        PyObject *o;
+    };
+
+    template <typename type>
+    static auto set_via_shared_from_this(type *value, std::shared_ptr<type> *holder_out)
+        -> decltype(value->shared_from_this(), bool()) {
+        // object derives from enable_shared_from_this;
+        // try to reuse an existing shared_ptr if one is known
+        if (auto existing = try_get_shared_from_this(value)) {
+            *holder_out = std::static_pointer_cast<type>(existing);
+            return true;
+        }
+        return false;
+    }
+
+    template <typename type>
+    static bool set_via_shared_from_this(void *, std::shared_ptr<type> *) {
+        return false;
+    }
+
+    template <typename type>
+    static bool set_foreign_holder(handle src, type *value, std::shared_ptr<type> *holder_out) {
+        // We only support using std::shared_ptr<T> for foreign T, and
+        // it's done by creating a new shared_ptr control block that
+        // owns a reference to the original Python object.
+        if (value == nullptr) {
+            *holder_out = {};
+            return true;
+        }
+        if (set_via_shared_from_this(value, holder_out)) {
+            return true;
+        }
+        *holder_out = std::shared_ptr<type>(value, py_deleter{src.inc_ref().ptr()});
+        return true;
+    }
+
+    template <typename type>
+    static bool
+    set_foreign_holder(handle src, const type *value, std::shared_ptr<const type> *holder_out) {
+        std::shared_ptr<type> holder_mut;
+        if (set_foreign_holder(src, const_cast<type *>(value), &holder_mut)) {
+            *holder_out = holder_mut;
+            return true;
+        }
+        return false;
+    }
+
+    template <typename type>
+    static bool set_foreign_holder(handle, type *, ...) {
+        throw cast_error("Unable to cast foreign type to held instance -- "
+                         "only std::shared_ptr<T> is supported in this case");
+    }
+};
+
 // SMART_HOLDER_BAKEIN_FOLLOW_ON: Rewrite comment, with reference to shared_ptr specialization.
 /// Type caster for holder types like std::shared_ptr, etc.
 /// The SFINAE hook is provided to help work around the current lack of support
@@ -907,6 +972,10 @@ struct copyable_holder_caster : public type_caster_base<type> {
         }
     }
 
+    bool set_foreign_holder(handle src) {
+        return holder_caster_foreign_helpers::set_foreign_holder(src, (type *) value, &holder);
+    }
+
     void load_value(value_and_holder &&v_h) {
         if (v_h.holder_constructed()) {
             value = v_h.value_ptr();
@@ -977,22 +1046,22 @@ struct copyable_holder_caster<
     }
 
     explicit operator std::shared_ptr<type> *() {
-        if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
+        if (sh_load_helper.was_populated) {
             pybind11_fail("Passing `std::shared_ptr<T> *` from Python to C++ is not supported "
                           "(inherently unsafe).");
         }
         return std::addressof(shared_ptr_storage);
     }
 
     explicit operator std::shared_ptr<type> &() {
-        if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
+        if (sh_load_helper.was_populated) {
             shared_ptr_storage = sh_load_helper.load_as_shared_ptr(typeinfo, value);
         }
         return shared_ptr_storage;
     }
 
     std::weak_ptr<type> potentially_slicing_weak_ptr() {
-        if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
+        if (sh_load_helper.was_populated) {
             // Reusing shared_ptr code to minimize code complexity.
             shared_ptr_storage
                 = sh_load_helper.load_as_shared_ptr(typeinfo,
@@ -1041,6 +1110,11 @@ struct copyable_holder_caster<
         }
     }
 
+    bool set_foreign_holder(handle src) {
+        return holder_caster_foreign_helpers::set_foreign_holder(
+            src, (type *) value, &shared_ptr_storage);
+    }
+
     void load_value(value_and_holder &&v_h) {
         if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
             sh_load_helper.loaded_v_h = v_h;
@@ -1078,6 +1152,7 @@ struct copyable_holder_caster<
                 value = cast.second(sub_caster.value);
                 if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
                     sh_load_helper.loaded_v_h = sub_caster.sh_load_helper.loaded_v_h;
+                    sh_load_helper.was_populated = true;
                 } else {
                     shared_ptr_storage
                         = std::shared_ptr<type>(sub_caster.shared_ptr_storage, (type *) value);
@@ -1224,6 +1299,12 @@ struct move_only_holder_caster<
         return false;
     }
 
+    bool set_foreign_holder(handle) {
+        throw cast_error("Foreign instance cannot be converted to std::unique_ptr "
+                         "because we don't know how to make it relinquish "
+                         "ownership");
+    }
+
     void load_value(value_and_holder &&v_h) {
         if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
             sh_load_helper.loaded_v_h = v_h;
@@ -1282,6 +1363,7 @@ struct move_only_holder_caster<
                 value = cast.second(sub_caster.value);
                 if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
                     sh_load_helper.loaded_v_h = sub_caster.sh_load_helper.loaded_v_h;
+                    sh_load_helper.was_populated = true;
                 } else {
                     pybind11_fail("Expected to be UNREACHABLE: " __FILE__
                                   ":" PYBIND11_TOSTRING(__LINE__));

include/pybind11/detail/type_caster_base.h

@@ -1058,6 +1058,7 @@ class type_caster_generic {
         return false;
     }
     void check_holder_compat() {}
+    bool set_foreign_holder(handle) { return true; }
 
     PYBIND11_NOINLINE static void *local_load(PyObject *src, const type_info *ti) {
         auto caster = type_caster_generic(ti);
@@ -1096,14 +1097,14 @@ class type_caster_generic {
     // logic (without having to resort to virtual inheritance).
     template <typename ThisT>
     PYBIND11_NOINLINE bool load_impl(handle src, bool convert) {
+        auto &this_ = static_cast<ThisT &>(*this);
         if (!src) {
             return false;
         }
         if (!typeinfo) {
-            return try_load_foreign_module_local(src);
+            return try_load_foreign_module_local(src) && this_.set_foreign_holder(src);
         }
 
-        auto &this_ = static_cast<ThisT &>(*this);
         this_.check_holder_compat();
 
         PyTypeObject *srctype = Py_TYPE(src.ptr());
@@ -1169,13 +1170,13 @@ class type_caster_generic {
         if (typeinfo->module_local) {
             if (auto *gtype = get_global_type_info(*typeinfo->cpptype)) {
                 typeinfo = gtype;
-                return load(src, false);
+                return load_impl<ThisT>(src, false);
             }
         }
 
         // Global typeinfo has precedence over foreign module_local
         if (try_load_foreign_module_local(src)) {
-            return true;
+            return this_.set_foreign_holder(src);
         }
 
         // Custom converters didn't take None, now we convert None to nullptr.
@@ -1189,7 +1190,7 @@ class type_caster_generic {
         }
 
         if (convert && cpptype && this_.try_cpp_conduit(src)) {
-            return true;
+            return this_.set_foreign_holder(src);
         }
 
         return false;

tests/local_bindings.h

@@ -65,11 +65,11 @@ PYBIND11_MAKE_OPAQUE(NonLocalMap)
 PYBIND11_MAKE_OPAQUE(NonLocalMap2)
 
 // Simple bindings (used with the above):
-template <typename T, int Adjust = 0, typename... Args>
-py::class_<T> bind_local(Args &&...args) {
-    return py::class_<T>(std::forward<Args>(args)...).def(py::init<int>()).def("get", [](T &i) {
-        return i.i + Adjust;
-    });
+template <typename T, int Adjust = 0, typename Holder = std::unique_ptr<T>, typename... Args>
+py::class_<T, Holder> bind_local(Args &&...args) {
+    return py::class_<T, Holder>(std::forward<Args>(args)...)
+        .def(py::init<int>())
+        .def("get", [](T &i) { return i.i + Adjust; });
 }
 
 // Simulate a foreign library base class (to match the example in the docs):

tests/pybind11_cross_module_tests.cpp

@@ -25,8 +25,9 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m, py::mod_gil_not_used()) {
     // relevant pybind11_tests submodule from a test_whatever.py
 
     // test_load_external
-    bind_local<ExternalType1>(m, "ExternalType1", py::module_local());
-    bind_local<ExternalType2>(m, "ExternalType2", py::module_local());
+    bind_local<ExternalType1, 0, std::shared_ptr<ExternalType1>>(
+        m, "ExternalType1", py::module_local());
+    bind_local<ExternalType2, 0, py::smart_holder>(m, "ExternalType2", py::module_local());
 
     // test_exceptions.py
     py::register_local_exception<LocalSimpleException>(m, "LocalSimpleException");

tests/test_local_bindings.cpp

@@ -22,6 +22,29 @@ TEST_SUBMODULE(local_bindings, m) {
     m.def("load_external1", [](ExternalType1 &e) { return e.i; });
     m.def("load_external2", [](ExternalType2 &e) { return e.i; });
 
+    struct SharedKeepAlive {
+        std::shared_ptr<int> contents;
+        int value() const { return contents ? *contents : -1; }
+        long use_count() const { return contents.use_count(); }
+    };
+    py::class_<SharedKeepAlive>(m, "SharedKeepAlive")
+        .def_property_readonly("value", &SharedKeepAlive::value)
+        .def_property_readonly("use_count", &SharedKeepAlive::use_count);
+    m.def("load_external1_shared", [](const std::shared_ptr<ExternalType1> &p) {
+        return SharedKeepAlive{std::shared_ptr<int>(p, &p->i)};
+    });
+    m.def("load_external2_shared", [](const std::shared_ptr<ExternalType2> &p) {
+        return SharedKeepAlive{std::shared_ptr<int>(p, &p->i)};
+    });
+    m.def("load_external2_unique", [](std::unique_ptr<ExternalType2> p) { return p->i; });
+
+    // Aspects of set_foreign_holder that are not covered:
+    // - loading a foreign instance into a custom holder should fail
+    // - we're only covering the case where the local module doesn't know
+    //   about the type; the paths where it does (e.g., if both global and
+    //   foreign-module-local bindings exist for the same type) should work
+    //   the same way (they use the same code so they very likely do)
+
     // test_local_bindings
     // Register a class with py::module_local:
     bind_local<LocalType, -1>(m, "LocalType", py::module_local()).def("get3", [](LocalType &t) {

tests/test_local_bindings.py

@@ -1,5 +1,8 @@
 from __future__ import annotations
 
+import sys
+from contextlib import suppress
+
 import pytest
 
 from pybind11_tests import local_bindings as m
@@ -20,6 +23,31 @@ def test_load_external():
         assert m.load_external1(cm.ExternalType2(12)) == 12
     assert "incompatible function arguments" in str(excinfo.value)
 
+    def test_shared(val, ctor, loader):
+        obj = ctor(val)
+        with suppress(AttributeError):  # non-cpython VMs don't have getrefcount
+            rc_before = sys.getrefcount(obj)
+        wrapper = loader(obj)
+        # wrapper holds a shared_ptr that keeps obj alive
+        assert wrapper.use_count == 1
+        assert wrapper.value == val
+        with suppress(AttributeError):
+            rc_after = sys.getrefcount(obj)
+            assert rc_after > rc_before
+
+    test_shared(110, cm.ExternalType1, m.load_external1_shared)
+    test_shared(220, cm.ExternalType2, m.load_external2_shared)
+
+    with pytest.raises(TypeError, match="incompatible function arguments"):
+        test_shared(210, cm.ExternalType1, m.load_external2_shared)
+    with pytest.raises(TypeError, match="incompatible function arguments"):
+        test_shared(120, cm.ExternalType2, m.load_external1_shared)
+
+    with pytest.raises(
+        RuntimeError, match="Foreign instance cannot be converted to std::unique_ptr"
+    ):
+        m.load_external2_unique(cm.ExternalType2(2200))
+
 
 def test_local_bindings():
     """Tests that duplicate `py::module_local` class bindings work across modules"""