-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Ownership of python objects inheriting from C++ classes #1389
Comments
This is, unfortunately, a limitation of pybind: we keep the C++ instance alive, but can't keep the Python instance alive. I think it might actually be possible to work around this with a custom holder. This is just brainstorming, but I think it might be possible to create something that works like a The downside is that it requires a custom holder class, i.e. not just a straight A second alternative is to store a Another approach (untested, but I think it ought to work), that might actually work for your specific case here, is to add a trampoline for class PyFoo : public Foo {
public:
PyFoo(std::shared_ptr<Vector> v) : Foo(v), pyobj(py::cast(v)) {}
private:
py::object pyobj;
}; and then bind the constructor using: py::class_<Foo, PyFoo>(m, "Foo")
.def(py::init_alias<std::shared_ptr<Vector>>())
; ( That way, the |
Thanks for the quick response, I have tried your workaround and it does indeed do the job. I do however have quite a large number of classes and functions that require objects like Regarding the custom holder: what would the implications of that be? Does that mean that the C++ code base that I'm wrapping around needs to be rewritten to use |
Just an idea here: In theory, a shared_ptr can hold any destructor ad a totally unrelated pointer. So it would definetely be possible to create a shared_ptr keeping the python object alive. So, something like this does work:
This does result in the correct output. I'm not sure though if you can add this in a way that keeps backward compability. Maybe only do this if the object in question is inherited in python. |
Oh that's neat! Would it be possible to combine this with a custom holder class to have this done automagically? The code below seems to work, but it would be great if I could just declare
|
If pybind already knows about the object then |
Don't do that: inheriting from most stl classes isn't a good idea (very few classes in the stl are polymorphic). Instead create a custom class with an |
Okay makes sense, I have tried this
but it fails with
I'm puzzled because I'm comparing to the ``custom_unique_ptr` in the tests and don't really see what I'm missing here... |
Okay so if I get rid of |
Got it to work with this implementation:
Does that make sense or might this cause issues somewhere internally in pybind? |
I think I had run into this issue in #1145 (which may also be related to #1333); I made an overly complicated fix for it #1146 which handles
This looks like it makes good sense for your applications. It may not release memory as quickly as you want it to, but I figure that'd be a small price to pay. We are using a variant of #1146 in a fork of |
That would be great! I'm sure other people have ran into similar issues and will find this useful as well. |
I've stumbled across this and have my own variant of basically the same idea #1546. I use the aliasing constructor to keep the Python object alive. It doesn't require a new holder type, just a tweak to the casting machinery. |
@cdyson37 Do you happen to have code available that has your change to the casting machinery? @pvallet's solution made me realize that I should've looked at what you said earlier - sorry! |
Ah, I see #1566 - sweet! |
This functionality requires the `shared_ptr` branch of libcommute. Unfortunately, pybind11 is not quite ready for this. pybind/pybind11#1389 pybind/pybind11#1566
Closing this in lieu of #1333 - please feel free to reopen if you think this is a unique case |
For anyone trying to use this as a workaround: this creates a reference cycle. This holder object refers to the Python object (via refcount) and the Python object holds a copy of this shared_ptr, so the shared_ptr's deleter will never be called. |
…ually by holding a reference to it See pybind/pybind11#1389 for why py_shared_ptr was needed in the first place, and the comment from May 27 why we may not want to use it (reference cycle)
…ually by holding a reference to it See pybind/pybind11#1389 for why py_shared_ptr was needed in the first place, and the comment from May 27 why we may not want to use it (reference cycle)
…ually by holding a reference to it See pybind/pybind11#1389 for why py_shared_ptr was needed in the first place, and the comment from May 27 why we may not want to use it (reference cycle)
…ually by holding a reference to it See pybind/pybind11#1389 for why py_shared_ptr was needed in the first place, and the comment from May 27 why we may not want to use it (reference cycle)
Hi,
I have an issue with the ref counting of python objects that are passed to C++.
I have a C++ class
Vector
that the use can inherit from (hence I also have aPyVector
trampoline class). The user-defined python class should then be passed back to C++ where some actions on this class are performed. Unfortunately, it seems that python takes complete ownership of the class, so that it gets deleted even if the C++ side still needs it.MFE:
Python code:
Output:
Expected output:
The text was updated successfully, but these errors were encountered: