Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Returning new instances of a derived type via a virtual factory function fails to keep the derived type alive #1774

Closed
yeswalrus opened this issue Apr 27, 2019 · 4 comments
Labels
holders Issues and PRs regarding holders

Comments

@yeswalrus
Copy link

Given a virtual function meant to return new instances of a type via a pointer to a base class, and a derived type declared in python, the return value cast won't keep the python object alive, resulting in a failure when attempting to call the virtual method on the new instance via the pointer. Because this function is called from c++, the return value policy has no effect.

Reproducible example code

https://github.com/yeswalrus/pybind11/tree/virtual-vector-return

@yeswalrus yeswalrus changed the title Returning new instances of a derived type via a virtual factory function Fails Returning new instances of a derived type via a virtual factory function fails to keep the derived type alive Apr 27, 2019
@eacousineau
Copy link
Contributor

eacousineau commented Apr 27, 2019

I believe this may be the 5th issue that tracks this problem :P
Ultimately, I think it's a form of Object Slicing.

Related issues:

I've worked around this in our fork of pybind11, but have not yet figured out a good way to make it useful for mainstream consumption. (The py::wrapper class it introduces is... well... a lot of blech.)

While my patch above works and is conservative for object lifetime, it may be costly (in object code and possibly runtime performance). Also, the above code is strongly coupled to also supporting unique_ptr, which also induces more overhead.

Simpler workarounds, suggested in the aforementioned issues, point towards making something like keep_alive_forever, which could be done by stashing py::object inside the Python trampoline class. In this fashion, Python may never delete until the interpreter shuts down, but it could be "deletable" by C++.

As far as issue tracking goes, I'll try to see which thread has the best recommendations, and see if we can consolidate the current duplicates.


EDIT (2020/06/17):
Also:

@yeswalrus
Copy link
Author

Ahh, sorry about the duplicate. Yeah, I dug into it in a debugger over the weekend a little bit - I was thinking something along the lines of making the trampoline class store a handle to the underlying python object would be a necessity.

@mmodenesi
Copy link

Reproducible example code

https://github.com/yeswalrus/pybind11/tree/virtual-vector-return

@yeswalrus what is the reproducible example, exactly? Thanks!

@EricCousineau-TRI
Copy link
Collaborator

Closing this in lieu of #1333 for tracking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
holders Issues and PRs regarding holders
Projects
None yet
Development

No branches or pull requests

4 participants