[BUG] Problem when creating derived Python objects from C++ (inheritance slicing)

[mhochsteger]

EDIT(eric): Adding a tracking list.

• Enumerate possible solutions and drawbacks, e.g.
  • Should investigate usage of py::wraper<> trampoline shim with GC garbage collection (to prevent inheritance slicing) #2757 Using py::wrapper<> and GC ressurrection
  • Others...
• Identify best path forward, implement.

---

Issue description

Hello,
Consider deriving a C++ class in Python and overloading virtual functions, including a 'Clone' function.
When passing a derived Python object to C++ without holding a reference (like calling Clone()), the information about the Python type is lost. In this case the wrong base-class function is called. More complex cases may result in a SEGFAULT.

Is there any way to work around/fix this without causing memory leaks?

Thanks!

Reproducible example code

test.cpp

// test.cpp
#include <pybind11/pybind11.h>
#include <Python.h>

#include <iostream>
#include <memory>

using std::cout;
using std::endl;
using std::unique_ptr;
using std::shared_ptr;
using std::make_shared;

namespace py = pybind11;

class Base {
public:
    virtual void Print() { 
        cout << "Base!" << endl;
    }

    virtual shared_ptr<Base> Clone() {
        return make_shared<Base>();
    }
};

class PyBase : public Base {
public:
    using Base::Base; // Inherit constructors
    void Print() override { PYBIND11_OVERLOAD(void, Base, Print, ); }
    shared_ptr<Base> Clone() override { PYBIND11_OVERLOAD(shared_ptr<Base>, Base, Clone, ); }
};

PYBIND11_MODULE(libtest, m) {
    py::class_<Base, py::wrapper<PyBase>, shared_ptr<Base>>(m, "Base")
      .def(py::init<>())
      ;

    m.def("Test", [] ( shared_ptr<Base> b ) {
          shared_ptr<Base> d = b->Clone();
          // Python object of d is already dead!
          d->Print();  // prints "Base!"
    });
}

test.py

from libtest import *

class Derived(Base):
    def __init__(self):
        super().__init__()

    def Print(self):
        print("Derived!")

    def Clone(self):
        return Derived()

Test(Derived())

[jagerman]

This seems a surprisingly hard problem. The main issue is that the C++ Clone and the Python Clone do fundamentally different things: the former returns a new C++ instance, the latter returns a new Python instance. The way referencing works is that the Python instance extends the C++ instance, rather than the other way around, which means casting to the C++ type isn't sufficient to keep the Python instance alive: you can easily end up being the only holder of the C++ instance, as happens here. (I don't think this will segfault: I think overload lookups will simply always fail in such a case since the owning object no longer exists).

The PyBase class could hold a py::object reference to the constructed object (you'd have to expand the PYBIND11_OVERLOAD and change it to save the auto o--which here is py::object), but then you've got a circular reference that can't be automatically cleaned up.

As I said, I don't see an easy way around this.

[yungyuc]

I worked it around by creating a manager class to hold the reference to the derived Python objects. It's ad hoc and ugly, but since I know exactly when to release the derived Python objects, it works for me.

I think this issue duplicates with #1145 . boost.python doesn't have this issue. I think this is a missing piece in pybind11.

[jagerman]

Agreed that it would be nice. (Retitling).

[EricCousineau-TRI]

Just curious (for some of my WIP PRs related to #1145): Can I ask if you know of an example boost.python code snippet that shows this working?

Also, do you happen to know where in the boost.python code this case is specifically handled? (preventing type slicing between C++ / Python?)

We are currently use a semi-hacky fork of pybind11 that handles this case for the Underactuated Robotics course / textbook; however, if there is a more elegant solution, I'm all for it.

[yungyuc]

I am not aware of an open source or self-containing example of it, but pretty sure boost.python works great. I didn’t trace that deep to boost.python to learn its mechanism for that feature. I’d guess it somehow holds the reference to the Python incarnation, since C++ types essentially is different from Python.

[EricCousineau-TRI]

Gotcha, thanks!

I've briefly skimmed through the Boost.Python source code for the component I think that's responsible, wrapper<T> + wrapper_base, and I do see self-referencing via PyObject*, but I have not yet seen any reference count increments to increase Python lifetime to match C++:
https://github.com/boostorg/python/blob/7352c9c0f770633e695aa8f48b647aa7a78e49c7/include/boost/python/detail/wrapper_base.hpp#L78
https://github.com/boostorg/python/blob/7352c9c0f770633e695aa8f48b647aa7a78e49c7/include/boost/python/object/pointer_holder.hpp#L200
Will see if I can dig a wee bit more to figure out if it does handle this, and maybe whip up a test case akin to what's in #1146.

[YannickJadoul]

Another possible solution was suggested here: #1049 (comment)

I'll close this for now, as the issue is +2 years old. Do reopen if necessary!