[C++-sig] Re: Pyste: Virtual class wrapper doesn't refcount object

[David Abrahams]

Dan Halbert <halbert at jubilee.bbn.com> writes:

>   >> Note that the self object is not properly reference-counted. It seems
>   >> to me it should be
>   >
>   >No, Pyste generates correct code for this case.  See
>   >http://www.boost.org/libs/python/doc/tutorial/doc/class_virtual_functions.html.
>
> OK, I see I am incorrectly blaming Pyste.
>
>   >The Python class already owns the C++ class. That would create an
>   >ownership cycle.
>   >
>   >> since it's held by the struct, and can disappear from scope in the
>   >> Python code, and so would go out of existence.
>
>
> OK, below is a complete tested small BPL example of the problem I had
> before I added the borrowed and handle<> in the callback wrapper. I
> must be misunderstanding something about object ownership.

Yep.

The basic principle is:

    A wrapped C++ object is always owned by the Python object that
    wraps it.

Another principle you can use is:

    A when a Python object x is converted to a shared_ptr<T> p, p
    maintains a reference to x, not just to *p.

I'm inverting the order of your code snippets:

| ==============================================================================
|
| Now, I'll try it in Python:
|
|
| % python
| Python 2.3.3 (#1, Mar  1 2004, 12:51:08) 
| [GCC 3.2.2] on linux2
| Type "help", "copyright", "credits" or "license" for more information.
|>>> import cb
|>>> class MyCallback(cb.Callback):     # subclass the C++ class
| ...     def do_callback(self):
| ...         print "callback called"
| ... 
|>>> cback = MyCallback()               # create an instance & remember it
|>>> cb.register_callback(cback)        # register the callback
|>>> cb.call_callback()                 # call the callback
| callback called                        # callback works!

Right; the callback's lifetime is maintained by the the module dict's
'cback' entry.

|>>> cb.register_callback(MyCallback()) # anonymous instance, short lifetime
|>>> cb.call_callback()                 # call the new callback instance
| Segmentation fault                     # BOOM!

Right; the Python object is gone at this point.

| ==============================================================================
| If I wrote
|
|   register_callback(MyCPPCallback()); // BAD IDEA; Callback doesn't live long

Right. I don't know what MyCPPCallback is, but if it's a class, you've
got the same problem as above.

| in C++, I wouldn't expect that to work at all, because the MyCPPCallback()
| object doesn't live past its use as a parameter. Maybe I shouldn't
| expect it to work in Python either? 

Not unless you do something to manage the lifetime of the Python object.

| I want the C++ code to take ownership of the MyCallback PyObject

Exactly.

| or more simply, just increment its reference count.

No, you don't want that.  In that case it'll leak.

| Perhaps I need a policy for the passed-in MyCallback object?? 

I don't see how it could help.

| The Call Policies part of the tutorial seems applicable. But all the
| appropriate-sounding policies seem to apply only to return values.
| What am I doing wrong?

The solution is to get the C++ code to hang onto the Python object,
rather than just the C++ object.  Here's one approach:

|
| ==============================================================================
| #include <boost/python.hpp>
| #include <boost/cstdint.hpp>
#include <boost/shared_ptr.hpp>

| using namespace boost::python;
|
| // Original C++ class
| struct Callback
| {
|     virtual void do_callback() = 0;
| };
|
| // BPL-tutorial-style wrapper, same as generated by Pyste
| struct Callback_Wrapper : Callback
| {
|     Callback_Wrapper(PyObject* self_) : Callback(), self(self_) {}
|     void do_callback() { call_method< void >(self, "do_callback"); }
|     PyObject* self;
| };

typedef boost::shared_ptr<Callback> CallbackPtr;

| // Callback* registered_callback;

CallbackPtr registered_callback;

| // void register_callback(Callback* cb)

void register_callback(CallbackPtr cb)
| {
|     registered_callback = cb;
| }
|
| void call_callback()
| {
|     registered_callback->do_callback();
| }
|
| BOOST_PYTHON_MODULE(cb)
| {
|     class_<Callback, boost::noncopyable, Callback_Wrapper>("Callback", init< >())
| 	.def("do_callback", pure_virtual(&Callback::do_callback));
|
|     def("register_callback", register_callback);
|     def("call_callback", call_callback);
| }
| ==============================================================================

HTH,

-- 
Dave Abrahams
Boost Consulting
www.boost-consulting.com