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

How to let C++ handle python instances of pybind11 derived types #1902

Closed
mmodenesi opened this issue Sep 1, 2019 · 5 comments
Closed

How to let C++ handle python instances of pybind11 derived types #1902

mmodenesi opened this issue Sep 1, 2019 · 5 comments

Comments

@mmodenesi
Copy link

mmodenesi commented Sep 1, 2019

Issue description

My use case is as follows:

Main project written in C++ lets users extend the codebase by subclassing a virtual CppClass and providing custom implementation of a virtual method. After registering a user_defined_factory_function, the application starts creating objects using a pointer to the polymorphic CppClass type, calling their method and finally deleting those objects in a way that is completely transparent to users. The whole point of the application is to load the user implementation of CppClass derived class.

I want to use pybind11 to allow users to subclass CppClass and override this method in python.

Supose the intepreter is initialized and finalized properly.

Supose the user factory function takes care of finding the python module and creating instances of the Python class deriving from CppClass. Then casts the py::object and reuturns a CppClass pointer.

The problem that I find is that the python objects are deleted as soon as the factory function returns. I need C++ to take ownership of the object. I repeat: the application takes care of deleting those objects when time comes.

I've read the documentation thoroughly. I've investigated all the questions tagged with pybind11 on SO. I've searched through the issues and I know this overlaps with:

Why do I add another issue, then?

I've explored the solutions proposed on other issues and couldn't fully understand them or tweak them to meet my needs. I am more of a python guy than a C++ guy, I'm trying to improve, though 😃 . In such vein, I think it would be useful to add a section to the documentation exploring the posibilities of using python objects on C++ in more involved ways, and explaining python vs. C++ objects lifecycle.

I think my issue adds value by providing a clear description of the use case as well as a complete example, exploring several posibilities.

Reproducible example code

Working example, just make and then ./program.

This is what I am pursuing:

[Python] __init__ called
[C++   ] end of user_defined_function
[C++   ] calling method on cpp pointer
[Python] method called
[Python] __del__ called
[C++   ]: destructor called

This is what I get:

[Python] __init__ called
[C++   ] end of user_defined_function
[Python] __del__ called
[C++   ]: destructor called
[C++   ] calling method on cpp pointer
Segmentation fault

If I manually inc_ref the underlying python object:

[Python] __init__ called
# inc_ref object here
[C++   ] end of user_defined_function
# good, no destructor called!
[C++   ] calling method on cpp pointer
[Python] method called
[C++   ] destructor called
# oops, should I worry python object no being collected?

More possibilities explored in the full example. The main idea, always, is to manually fiddle with ref_count. Is this too hacky?

@sizmailov
Copy link
Contributor

sizmailov commented Sep 1, 2019

You are right, you need to keep PyClass instance ref count from going to zero. In addition you want to destroy python part of the object through a raw pointer CppClass *.

(option 1) You can archive it by storing py::object inside of CppClass-derived class PyClassWrapper. py::object automatically does incref/decref on underlying python object.

Note: in this approach you will have two CppClass instances per PyClass (one is 'real' one, another one is wrapper around python object) to distinguish between two types of CppClass* pointers: with and without python part.

Note: you need to make CppClass:~CppClass() virtual, otherwise destructor-of-derived-type would NOT be called though a pointer-to-base-class.

someclass.h:

class CppClass {
public:
  CppClass() = default;
  virtual void method();
  virtual ~CppClass(); // must be virtual! 
};

Corresponding changes to main.cc would be

/**
* Implements CppClass interface by calling stored py::object.
* Stored py::object extends python object's lifetime. 
*/
class PyClassWrapper : public CppClass {
public:
  PyClassWrapper(py::object pyClass): m_pyClass(std::move(pyClass)){}
  virtual ~PyClassWrapper() = default;
  virtual void method(){
      m_pyClass.attr("method")();
  }
private:
  py::object m_pyClass;
};

CppClass* user_defined_function(bool incref) {
    py::module module = py::module::import("module");
    py::object obj = module.attr("PyClass")();
    if (incref) { obj.inc_ref(); } 
    log("end of user_defined_function");
    return new PyClassWrapper(obj); // implicit cast to CppClass*
}
The output of main:
[C++   ] TEST 1: no factory function
[Python] __init__ called on <module.PyClass object at 0x7f45fe9ddca8> 1
[C++   ] calling method on python object
[Python] method called on <module.PyClass object at 0x7f45fe9ddca8> 1
[C++   ] calling method on cpp pointer
[Python] method called on <module.PyClass object at 0x7f45fe9ddca8> 1
[C++   ] end of scoped code
[Python]  __del__ called on <module.PyClass object at 0x7f45fe9ddca8> 1
[C++   ] destructor called [0x562833e1ab70]

[C++   ] TEST 2: no incref
[Python] __init__ called on <module.PyClass object at 0x7f45fe9ddca8> 2
[C++   ] end of user_defined_function
[C++   ] calling method on cpp pointer
[Python] method called on <module.PyClass object at 0x7f45fe9ddca8> 2
[Python]  __del__ called on <module.PyClass object at 0x7f45fe9ddca8> 2    // destruction of python part of PyClass instance 
[C++   ] destructor called [0x562833e1ab90]   // destruction of c++ part of PyClass instance 
[C++   ] destructor called [0x562833e1aaf0]   // destruction of c++ part of PyClassWrapper instance 

[C++   ] TEST 3 : incref
[Python] __init__ called on <module.PyClass object at 0x7f45fe9ddca8> 3
[C++   ] end of user_defined_function
[C++   ] calling method on cpp pointer
[Python] method called on <module.PyClass object at 0x7f45fe9ddca8> 3
[C++   ] deleting instance3
[C++   ] destructor called [0x562833e1ab90]

[C++   ] TEST 4 : incref and decref
[Python] __init__ called on <module.PyClass object at 0x7f45fe9ddd00> 4
[C++   ] end of user_defined_function
[C++   ] calling method on cpp pointer
[Python] method called on <module.PyClass object at 0x7f45fe9ddd00> 4
[C++   ] decrementing ref count on instance4 wrapped python object

[C++   ] TEST 5 : incref and decref twice
[Python] __init__ called on <module.PyClass object at 0x7f45fe9ddd58> 5
[C++   ] end of user_defined_function
[C++   ] calling method on cpp pointer
[Python] method called on <module.PyClass object at 0x7f45fe9ddd58> 5
[C++   ] decrementing ref count on instance5 wrapped python object (twice)
[C++   ] End of main

(option 2) You can keep table of PyClass pointers along with corresponding CppClass pointers on C++ side and do incref/decref manually. This is more intrusive and requires C++ library code changes.

The first method is simpler to implement correctly, but second one should be more efficient on frequent method calls (supposedly). I would measure overhead of first method and decide whether it would be worth to implement second one.

@mmodenesi
Copy link
Author

@sizmailov Thank your very much for your detailed answer. I am taking some time to think about the solutions you suggest. I'll be coming back soon.

@mmodenesi
Copy link
Author

@sizmailov I am going for the simple approach of incrementing ref_count when I create the python derived instances (I have total control over that) and decrementing ref_count on the class destructor (I know the main application does delete instances). I use py::nodelete to prevent the destructor of the python object from triggering the destructor of the cpp class a second time.

So far, it's working.

Thank you again for the detailed answer. Feel free to close the issue.

@sizmailov
Copy link
Contributor

@mmodenesi I have no permissions to close issues. You can close it yourself (as issue author).

@EricCousineau-TRI
Copy link
Collaborator

Most likely a duplicate of #1333 (as the OP indicated by reference to #1389) - just making a transitive closure 😬

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

No branches or pull requests

3 participants