[C++-sig] Re: Problem: held type, std::auto_ptr, ReferenceErrors.

[David Abrahams]

Prabhu Ramachandran <prabhu at aero.iitm.ernet.in> writes:

> Hi Dave,
>
>>>>>> "DA" == David Abrahams <dave at boost-consulting.com> writes:
>
>     DA> The following message is a courtesy copy of an article that
>     DA> has been posted to gmane.comp.python.c++ as well.
>
> Thanks for the followup and the CC!
>
>     >> Problem 1:
>
>     DA> If you pass this through c++filt you'll see that it has a
>     DA> Python object of type (Python) B but can't match it to a C++
>     DA> function requiring a (C++) B lvalue argument.  That can only
>     DA> mean that there's no C++ B object in the Python B object.
>     DA> Given that you're holding these things by auto_ptr, which
>     DA> gives up ownership when copied, it's a strong hint that the
>     DA> object has been passed where a C++ std::auto_ptr object was
>     DA> expected, e.g. to your add_wrapper function.
>
> IIUC, you are saying that the object was not held correctly by
> std::auto_ptr?  

What do you mean by "correctly?"

I was suggesting that it was held (correctly), and then the object's
ownership was (correctly?) passed off to the auto_ptr which was used
as an argument to a function.  Ownership passing when auto_ptrs are
copied is a feature of auto_ptr, so in that sense it's "correct" to
pass ownership.  But I can't say whether that's the behavior you
wanted.

> I can confirm that add_wrapper was indeed being called
> because I added a print statement after that.  Boost.Python
> (correctly) complains if I pass the object itself 
                                  ^^^^^^^^^^^^^^^^^
you have to be more specific; there are lots of objects flying
around.  The C++ object? The Python object? From C++ or from Python?
A code example helps.

> to add_wrapper so I can't see how the object itself was passed to
> add_wrapper and that the h.add(b) call worked.  I probably am not
> understanding what you mean but please do read on, I have another
> example (hopefully simpler).
>
>     >> If I remove the HeldType and leave it as it is, everything
>     >> works fine upto the func(x) call that is.
>
>     DA> And then... what?
>
> And then problem 2.
>
>     >> I also tried by specifying a boost::shared_ptr<T> as the held
>     >> type.  I did not need to use the add_wrapper in these two
>     >> cases.  That also seems to work just fine.
>
>     DA> It sounds like you're very confused about object ownership
>     DA> issues in all these cases.
>
> Well, I was giving you as many data points as I could in order that
> you might get a clearer picture of what is going on.  

If you want help, the best strategy is to give as few data points as
possible, i.e. reduce each distinct problem to a separate minimal test
case.

> Here is what I understand of the object ownership issues.  I could
> be wrong but I guess its best that I get it cleared up at the
> earliest.
>
> [ example based on code posted in last email ]
>>>> b = B()
>
> The Python object b holds a pointer to a B_Wrapper instance via
> std::auto_ptr<B_Wrapper>.

I would say that the Python object b holds a std::auto_ptr<B_Wrapper>
which holds a B_Wrapper instance.

>>>> h = Holder()
>>>> h.add(b)
>
> h.add calls the defined add_wrapper which expects a std::auto_ptr<B>.
> Since I've declared std::auto_ptr<B_Wrapper> to be implicitly
> convertible to std::auto_ptr<B>, when 'b' is passed to the add_wrapper
> function, the add_wrapper gets the pointer to the B_Wrapper, and
> releases b's auto_ptr that holds B_Wrapper.  So the b object will not
> be useable anymore.  

Good so far.

> However h.get gets me the pointer that is stored in h:
>
>>>> x = h.get(0)
>>>> x.f()
> B::f

Well, it gets you a Python 'A' object which "holds" its C++ object
with a raw pointer (i.e. no ownership).

> Which clearly appears to work confirming what I was saying above.
> However I thought that func(x) should also work, but it does not and
> produced the error I showed earlier.  

It looks as though that should work, to me.  But I can't see the
wrapping code.

> This is what I think is going on.  I could certainly be wrong and
> would appreciate if you can clarify where I am wrong.

I can't tell much without the wrapping code.

>     >> So this looks to be a bug similar to the one earlier where
>     >> x.f() used to fail when the held type was std::auto_ptr<T>.
>     >> AFAIK, this does not look to be a problem with Pyste.
>
>     DA> If you really think this is a bug in Boost.Python itself then
>     DA> please reduce the Pyste-generated code and associated Python
>     DA> to a minimal case so we can see what's really going on.
>
> OK, I just did that.  Here is an example.

<snip>

> // ----- bug1.pyste -----
> def held_type_func(name):
>     return 'std::auto_ptr< %s >'%name

<snip>


This is not minimal.  I want to see the C++ wrapping code.  Believe
it or not I have never even run Pyste.

>     >> Problem 2:
> [snip]
>     >> I also tried to use a return_internal_reference<1>(); instead
>     >> of the return_value_policy(reference_existing_object)) and get
>     >> the same results.  I also changed the HBase* argument to a
>     >> Holder* argument with the same results.
>
>     DA> Sounds like you're just flailing about here; think about what
>     DA> actually goes on instead.  The HBase* passed to f actually
>
> Sorry, my intent was to give you as much information as I could.

I don't mind having the information; I'm just saying that you appear
to be trying changes to the code somewhat at random, which seldom
produces good results even when it appears to "work".

>     DA> points to a Holder_Wrapper generated by Pyste so that you can
>     DA> override a Holder's virtual functions in Python.  Its get()
>     DA> implementation looks up a Python get method and calls that.
>     DA> Holder's get() function returns a new Python object (which
>     DA> simply points at the referenced object), to which there is
>     DA> only one reference.  The dangling reference detector doesn't
>     DA> know anything about the fact that this object is merely
>     DA> pointing at some existing object - it assumes the Python
>     DA> object has ownership of the C++ object it holds, and that C++
>     DA> object will die when the Python object does.
>
> Many thanks for the clarification!  I still have doubts, sorry, please
> bear with me.
>
> All this happens at the h->get(i)->f() call.  So h->get(i) gives me
> this new Python object that points to the referenced object.  

Ahh, no.  h->get(i) is C++ syntax, so it doesn't give you a Python
object... unless it returns python::object (which it doesn't).
h->get(i) returns a pointer to a bare A* object.

> The lifetime of this new Python object is guaranteed till the end of
> the statement h->get(i)->f()

No, the lifetime of the A* ("pa") is guaranteed to the end of the
statement, but that doesn't mean the *pa is valid.

> , so why should this be an error?  

I think you'll need to look at the Pyste-generated code to understand
what's happening.

> The Python object goes away only at the end of the statement.  

Nope.

> Also how is this different from doing this on the Python
> interpreter?

In too many ways to describe.

>>>> print h.size()
> 1
>>>> for i in range(h.size()):
> ...  h.get(i).f()
> ... 
> B::f
>
> If this is legal, why is it illegal when a function does the exact
> same thing in C++?

Here's a near-minimal case which I hope will illustrate:

struct Foo
{
   virtual Foo* f()  // 4. wrapped as "Foo.f"
   {
       return &foo;
   }                 // 5. Wrapper using reference_existing_object
                     // builds a new Python Foo object with one
                     // reference count

   static Foo foo;
};
Foo Foo::foo;

struct Foo_Wrapper : Foo
{
   Foo_Wrapper(PyObject* self)
     : m_self(self) {}

   Foo* f()
   { 
       // 3. calls into Python
       return call_method<Foo*>(m_self, "f"); 

       // 6. call_method looks at the Python Foo object it just got
       //    and says "I know you're about to access a pointer into
       //    this object.  How many references does it have? Oh, 1?
       //    In that case it will be destroyed before I return.
       //    Sorry, that pointer will be invalid.  Exception!"
   }
   PyObject* m_self;
};

void func(Foo* x)  // 1. x actually will point to a Foo_Wrapper
{
    x.f();         // 2. calls Foo_Wrapper::f
}

BOOST_PYTHON_MODULE(foo)
{
    def("func", func);

    class_<Foo,Foo_Wrapper>("Foo")
        .def(
             "f"
           , &Foo::f
           , &Foo_Wrapper::f
           , return_value_policy<reference_existing_object>()
        )
    ;
}


Python:

    >>> fu = Foo()
    >>> fu2 = fu.f()
    >>> func(fu2)


> One thought is that when objects are returned via a return value
> policy you could tag it as such and be a little more lenient on
> these or handle these specially.  

Yes, it's a possibility.

> However, I am not sure of what I'm talking about anymore given my
> flaky understanding of various things.

If you weren't using Pyste, you could do something like:

struct Foo_Wrapper : Foo
{
   Foo_Wrapper(PyObject* self)
     : m_self(self) {}

   Foo* f()
   { 
       object self(handle<>(borrowed(m_self)));
       return extract<Foo*>(self.attr("f")());
   }
   PyObject* m_self;
};


> BTW, SWIG, provides a lower level interface (in that you can trigger
> a segfault on the interpreter if you know what to do) but it does
> work nicely with code like the above.  It leaves memory management
> issues to the user which is why you can shoot yourself.  

Yeah, letting a C++ user shoot himself is fine, but letting a Python
user shoot himself goes against the Boost.Python philosophy.

> However, the user can also change the shadow classes so that the
> interface is much cleaner and memory management issues can be
> handled quite safely by changing the thisown attribute
> appropriately.

Sure, you can do almost anything you want with Boost.Python by
writing C++ wrapper layers over your code, too.

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