[BUG]: pybind11 should not expect there is always a shared_ptr when detecting enable_shared_from_this

[roastduck]

Required prerequisites

• Make sure you've read the documentation. Your issue may be addressed there.
• Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
• Consider asking first in the Gitter chat room or in a Discussion.

Problem description

Currently, when pybind11 detects a class inheriting std::enable_shared_from_this, it asserts users always uses std::shared_ptr to hold it. Specifically, it checks std::shared_ptr::element_type at this line:

pybind11/include/pybind11/pybind11.h

Line 1784 in 47079b9

auto sh = std::dynamic_pointer_cast<typename holder_type::element_type>(

However, even when a class inherits std::enable_shared_from_this, an object of its does not necessarily be held in a std::shared_ptr. It can be held by std::shared_ptr in just some part of the code, and held by other holders when passing to pybind11. In my case, I warped around std::shared_ptr to build my own holder, which does not have an element_type member type.

A possible fix is to check the type of holder_type first before assuming it is a std::shared_ptr.

Reproducible example code

No response

[Skylion007]

@roastduck, if you have a fix in mind a PR would be appreciated. @rwgk something to consider here for smart holder casting.

[rwgk]

@roastduck, did you already try the smart_holder branch, using classh for the type(s) that give you trouble on master?

https://github.com/pybind/pybind11/blob/smart_holder/README_smart_holder.rst

A reproducer would be useful (draft PR).

If it doesn't work with the smart_holder branch, I'll look into fixing.

[roastduck]

@Skylion007, in my understanding, changing the following declaration

pybind11/include/pybind11/pybind11.h

Lines 1778 to 1782 in 47079b9

	template <typename T>
	static void init_holder(detail::instance *inst,
	detail::value_and_holder &v_h,
	const holder_type * /* unused */,
	const std::enable_shared_from_this<T> * /* dummy */) {

to

template <typename T, typename U, typename V,
          typename std::enable_if_t<std::is_base_of_v<std::shared_ptr<V>, U>> * = nullptr> 
 static void init_holder(detail::instance *inst, 
                         detail::value_and_holder &v_h, 
                         const U * /* unused */, 
                         const std::enable_shared_from_this<T> * /* dummy */) {

will work. But I am not sure how to safely write these template arguments for maximal compatibility.

I am also not sure how to write a test in pybind11, but I come up with a reproducible code, as follows:

#include <pybind11/pybind11.h>

#include <memory>

template <class T>
class Ref {
    std::shared_ptr<T> ptr_;

   public:
    Ref(const std::shared_ptr<T> &ptr) : ptr_(ptr) {}
    Ref(T *ptr) : ptr_(ptr) {}

    T &operator*() const { return *ptr_; }
    T *operator->() const { return ptr_.get(); }
    T *get() const { return ptr_.get(); }
};

class A : public std::enable_shared_from_this<A> {  // comment this line
    // class A {// and uncomment this line and then it compiles
    int x_;
};

PYBIND11_DECLARE_HOLDER_TYPE(T, Ref<T>);

PYBIND11_MODULE(my_module, m) {
    pybind11::class_<A, Ref<A>>(m, "A").def(pybind11::init<>());
}

It will not compile unless you remove : public std::enable_shared_from_this<A>.

Could you make a PR like this?

@rwgk, I have not tried smart_holder yet, because this problem is related to a custom pointer, while the README of smart_holder says it "does not have a well-lit path for including interoperability with custom smart-pointers".

[rwgk]

will work. But I am not sure how to safely write these template arguments for maximal compatibility.

"None of us is" is probably much closer to the truth than "we (the maintainers) are".
At least speaking for myself, I usually try-fail-try until I have what I need. Recommended here.

"does not have a well-lit path for including interoperability with custom smart-pointers"

Something that crossed my mind, but it's just a wild idea atm, is to convert the custom smart-pointer to a shared_ptr with custom deleter, from there the smart_holder may work as-is. — Do you have a pointer to your code?