type_caster_generic: add set_foreign_holder method for subclasses to implement (#5862)

* type_caster_generic: add set_foreign_holder method for subclasses to implement * style: pre-commit fixes * Rename try_shared_from_this -> set_via_shared_from_this to avoid confusion against try_get_shared_from_this * Add comment explaining the limits of the test * CI * style: pre-commit fixes * Fixes from code review * style: pre-commit fixes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-03-14 20:27:47 +00:00 · 2025-10-13 20:00:28 -04:00
parent 9f75202191
commit c7b4f66a73
10 changed files with 186 additions and 16 deletions
--- a/.codespell-ignore-lines
+++ b/.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;
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -188,6 +188,7 @@ set(PYBIND11_HEADERS
    include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h
    include/pybind11/detail/exception_translation.h
    include/pybind11/detail/function_record_pyobject.h
+    include/pybind11/detail/holder_caster_foreign_helpers.h
    include/pybind11/detail/init.h
    include/pybind11/detail/internals.h
    include/pybind11/detail/native_enum_data.h
--- a/include/pybind11/cast.h
+++ b/include/pybind11/cast.h
@@ -13,6 +13,7 @@
 #include "detail/argument_vector.h"
 #include "detail/common.h"
 #include "detail/descr.h"
+#include "detail/holder_caster_foreign_helpers.h"
 #include "detail/native_enum_data.h"
 #include "detail/type_caster_base.h"
 #include "detail/typeid.h"
@@ -907,6 +908,10 @@ protected:
        }
    }

+    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,7 +982,7 @@ public:
    }

    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).");
        }
@@ -985,14 +990,14 @@ public:
    }

    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 +1046,11 @@ protected:
        }
    }

+    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 +1088,7 @@ protected:
                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 +1235,12 @@ public:
        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 +1299,7 @@ public:
                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__));
--- a/include/pybind11/detail/holder_caster_foreign_helpers.h
+++ b/include/pybind11/detail/holder_caster_foreign_helpers.h
@@ -0,0 +1,86 @@
+/*
+    pybind11/detail/holder_caster_foreign_helpers.h: Logic to implement
+    set_foreign_holder() in copyable_ and movable_holder_caster.
+
+    Copyright (c) 2025 Hudson River Trading LLC <opensource@hudson-trading.com>
+
+    All rights reserved. Use of this source code is governed by a
+    BSD-style license that can be found in the LICENSE file.
+*/
+
+#pragma once
+
+#include <pybind11/gil.h>
+
+#include "common.h"
+
+PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
+PYBIND11_NAMESPACE_BEGIN(detail)
+
+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");
+    }
+};
+
+PYBIND11_NAMESPACE_END(detail)
+PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
--- a/include/pybind11/detail/type_caster_base.h
+++ b/include/pybind11/detail/type_caster_base.h
@@ -1069,6 +1069,7 @@ public:
        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);
@@ -1107,14 +1108,14 @@ public:
    // 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());
@@ -1180,13 +1181,13 @@ public:
        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.
@@ -1200,7 +1201,7 @@ public:
        }

        if (convert && cpptype && this_.try_cpp_conduit(src)) {
-            return true;
+            return this_.set_foreign_holder(src);
        }

        return false;
--- a/tests/extra_python_package/test_files.py
+++ b/tests/extra_python_package/test_files.py
@@ -83,6 +83,7 @@ detail_headers = {
    "include/pybind11/detail/descr.h",
    "include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h",
    "include/pybind11/detail/function_record_pyobject.h",
+    "include/pybind11/detail/holder_caster_foreign_helpers.h",
    "include/pybind11/detail/init.h",
    "include/pybind11/detail/internals.h",
    "include/pybind11/detail/native_enum_data.h",
--- a/tests/local_bindings.h
+++ b/tests/local_bindings.h
@@ -25,8 +25,9 @@ using MixedLocalGlobal = LocalBase<4>;
 using MixedGlobalLocal = LocalBase<5>;

 /// Registered with py::module_local only in the secondary module:
-using ExternalType1 = LocalBase<6>;
-using ExternalType2 = LocalBase<7>;
+using ExternalType1 = LocalBase<6>; // default holder
+using ExternalType2 = LocalBase<7>; // held by std::shared_ptr
+using ExternalType3 = LocalBase<8>; // held by smart_holder

 using LocalVec = std::vector<LocalType>;
 using LocalVec2 = std::vector<NonLocal2>;
@@ -65,11 +66,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):
--- a/tests/pybind11_cross_module_tests.cpp
+++ b/tests/pybind11_cross_module_tests.cpp
@@ -26,7 +26,9 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m, py::mod_gil_not_used()) {

    // test_load_external
    bind_local<ExternalType1>(m, "ExternalType1", py::module_local());
-    bind_local<ExternalType2>(m, "ExternalType2", py::module_local());
+    bind_local<ExternalType2, 0, std::shared_ptr<ExternalType2>>(
+        m, "ExternalType2", py::module_local());
+    bind_local<ExternalType3, 0, py::smart_holder>(m, "ExternalType3", py::module_local());

    // test_exceptions.py
    py::register_local_exception<LocalSimpleException>(m, "LocalSimpleException");
--- a/tests/test_local_bindings.cpp
+++ b/tests/test_local_bindings.cpp
@@ -21,6 +21,31 @@ TEST_SUBMODULE(local_bindings, m) {
    // test_load_external
    m.def("load_external1", [](ExternalType1 &e) { return e.i; });
    m.def("load_external2", [](ExternalType2 &e) { return e.i; });
+    m.def("load_external3", [](ExternalType3 &e) { return e.i; });
+
+    struct SharedKeepAlive {
+        std::shared_ptr<int> contents;
+        int value() const { return contents ? *contents : -20251012; }
+        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_external2_shared", [](const std::shared_ptr<ExternalType2> &p) {
+        return SharedKeepAlive{std::shared_ptr<int>(p, &p->i)};
+    });
+    m.def("load_external3_shared", [](const std::shared_ptr<ExternalType3> &p) {
+        return SharedKeepAlive{std::shared_ptr<int>(p, &p->i)};
+    });
+    m.def("load_external1_unique", [](std::unique_ptr<ExternalType1> p) { return p->i; });
+    m.def("load_external3_unique", [](std::unique_ptr<ExternalType3> 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:
--- a/tests/test_local_bindings.py
+++ b/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
@@ -11,6 +14,7 @@ def test_load_external():

    assert m.load_external1(cm.ExternalType1(11)) == 11
    assert m.load_external2(cm.ExternalType2(22)) == 22
+    assert m.load_external3(cm.ExternalType3(33)) == 33

    with pytest.raises(TypeError) as excinfo:
        assert m.load_external2(cm.ExternalType1(21)) == 21
@@ -20,6 +24,36 @@ 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(220, cm.ExternalType2, m.load_external2_shared)
+    test_shared(330, cm.ExternalType3, m.load_external3_shared)
+
+    with pytest.raises(TypeError, match="incompatible function arguments"):
+        test_shared(320, cm.ExternalType2, m.load_external3_shared)
+    with pytest.raises(TypeError, match="incompatible function arguments"):
+        test_shared(230, cm.ExternalType3, m.load_external2_shared)
+
+    with pytest.raises(
+        RuntimeError, match="Foreign instance cannot be converted to std::unique_ptr"
+    ):
+        m.load_external1_unique(cm.ExternalType1(2200))
+
+    with pytest.raises(
+        RuntimeError, match="Foreign instance cannot be converted to std::unique_ptr"
+    ):
+        m.load_external3_unique(cm.ExternalType3(3300))
+

 def test_local_bindings():
    """Tests that duplicate `py::module_local` class bindings work across modules"""