[C++-sig] Re: boost::python and threads

[David Abrahams]

Vladimir Vukicevic <vladimir at pobox.com> writes:

> David Abrahams wrote:
>
>  >>FooProxy is a remote object that I get as a result of calling various
>  >>functions to get a proxy -- it has a "void bar();" (non-virtual) that
>  >>I can call. >
>  >
>  >Hmm.  This is totally off-topic, but it seems odd to me (and
>  >inconvenient, in C++) that FooProxy is not derived from Foo.
>  >
> Live servants and proxies live in different class hierarchies; the
> Proxy objects just know how to add their tag to a packet and serialize
> arguments to operations, whereas Foo might have additional data
> members and the like.  

Which is why Foo ought to be an empty abstract base class.  But anyway...

>  >>It's this problem that I'm directly dealing with -- I can't figure
>  >>out a way to wrap class FooProxy such that I can manage the current
>  >>ThreadState and the GIL around them.  I can create another wrapper
>  >>class
>  >
>  >Do you mean like a virtual-function-dispatching class, the kind with
>  >the initial "PyObject* self" argument to its constructors?
>
> Yes.
>
>  >>that does the right thing and calls FooProxy::bar(), but
>  >>functions are returning a FooProxy (not mywrapper_for_FooProxy)
>  >
>  >
>  >I thought you said that all the C++ interfaces passed FooProxyHandle
>  >(?)
>
> Sorry, my mistake.. it is a FooProxyHandle.  Internally FooProxy is
> typedef'd to ::Ice::ProxyHandle< ::IceProxy::Foo> :)

See boost/python/converter/register_ptr_to_python.hpp in the CVS.

>  >>and I already have a Handle<FooProxy> as a wrapper specified in the
>  >>boost::python class_... so I end up with incorrect types half the
>  >>time.
>  >
>  >
>  >Specifically, what problem are you having?  Could you show a function
>  >you are unable to wrap properly and describe the specific problems
>  >you're having?
>
> Sure; see end of message, as it's somewhat lengthy and I didn't want
> to clutter up the replies. (*)
>
>  >>However, an alternative solution that I was thinking of is to bake the
>  >>thread smarts directly into boost::python. >
>  >
>  >I have always thought that should happen in some form or other anyway.
>  >
>  >>It seems to me that call_method and similar should always be wrapped
>  >>with a PyGILState_Ensure/PyGILState_Release. >
>  >
>  >Some people know their extensions are only being called on the main
>  >thread and not everyone will be ready to pay the cost for acquiring
>  >the GIL on every callback.  IMO supplying a GILState class which
>  >acquires on construction and releases on destruction *should* be
>  >enough.
>
> Hmm, I'm not sure what you mean; where would you apply the GILState
> class?  I agree though, acquiring the GIL on every callback is
> heavyhanded, but necessary for some applications (like mine, where I
> don't know exactly which thread a callback will get invoked on for a
> particular object instance).

I guess from your other messages you figured that out.

>  >>Also, an additional call policy could be added, something like
>  >>"allow_threads", which would cause b::p to call
>  >>Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS around the function
>  >>invocation on C++.  This seems like a much more generally useful
>  >>solution than me trying to do the dance in wrapper classes.
>  >
>  >I think some change to the requirements and usage of CallPolicies
>  >would be neccessary in order to support that (see my reply to
>  >Nikolay), but I agree with the general idea.
>  >
>  >>Is this the right way to go? >
>  >Probably.  I also think that some people want automatic
>  >Py_BEGIN_ALLOW_THREADS/Py_END_ALLOW_THREADS for *all* of their wrapped
>  >functions, and that should probably be configurable, too.  You have to
>  >be careful with that, though: once you do Py_BEGIN_ALLOW_THREADS you
>  >can't use any Boost.Python facilities which use the Python 'C' API
>  >(e.g. object, handle<>, ...) until you reach Py_END_ALLOW_THREADS.
>
> Yep; I assume invoke.hpp is the lowest-level at which the C++
> function is actually invoked.  

Close.

>  >>If so, I'd appreciate any pointers on implementing a new call
>  >>policy, especially if it's possible to both specify allow_threads
>  >>and a return_value_policy.
>  >
>  >I think you'd better start by looking at the macro definition of
>  >Py_BEGIN_ALLOW_THREADS et al and then at how policies are used to see
>  >how state can be stored for the duration of the call they affect, as
>  >the macros are wont to do.
>
> * Specific function problem:
>
> Each ProxyHandle<Proxy::Foo> exports (via the ProxyHandle class) a
> checkedCast() static member, that can be used to convert
> ProxyHandle<Proxy::Object> (which is the root of all proxies) to a
> specific proxy type. checkedCast() checkedcast actually creates a new
> ProxyHandle<Proxy::Foo> object, and then copies the target server
> information into it from the ProxyHandle<Proxy::Object>, so there's no
> "cast" in the C++ sense going on (though it does check if it can
> dynamic_cast<> the Proxy::Object* to a Proxy::Foo*, and if so, just
> does that).
>
> To have support for checkedCast in python, I create wrapper functions
> such as:
>
> namespace pyce_Hello_casts {
> ::IceInternal::ProxyHandle< ::IceProxy::Hello> _Hello_checkedCast
                                                 ^^^^^^^^^^^^^^^^^^
This name is reserved for your C++ implementation.

> (const ::Ice::ObjectPrx& o)
> {
>     return ::IceInternal::ProxyHandle< ::IceProxy::Hello>::checkedCast (o);
> }
> };
>
> and my class_<> def looks like:
>
> class_< ::IceProxy::Hello ,
>        ::IceInternal::ProxyHandle< ::IceProxy::Hello >,
>        bases< ::IceProxy::Ice::Object >,
>        boost::noncopyable >
>    ("HelloPrx")
>  .def("sayHello", &::IceProxy::Hello::sayHello)
>  .def("checkedCast", pyce_Hello_casts::_Hello_checkedCast)
>  .staticmethod("checkedCast")
> ;
>
> This all works fine, until I want to intercept the call to sayHello()
> (to save/restore the ThreadState goop).  I can create a wrapper that
> derives from ::IceProxy::Hello, but then I have to deal with that
> PyObject* constructor, for which I have no need for here -- I'm never
> going to utilize call_method, and HelloPrx will never be derived from
> in python-land.  The sayHello() and other member functions are also
> not virtual.  I could do a dance with a custom-written overload
> dispatch for each function.  If I went the wrapper route, I would need
> to implement my own version of checkedCast that can convert to my
> wrapper, and add some implicitly_convertible statements, and make sure
> that no references to ::IceProxy::Hello (or the equivalent handle) get
> exposed anywhere.. certainly feasable, but a pain, especially for
> cases where I get a handle to a ::IceProxy::Hello in C++ and I'd have
> to convert.
>
> But both approaches seem like they're far too verbose.  I'll probably
> create a call_method_with_threads<> such that the non-GIL-acquiring
> call_method<> is still available; extending call policies would allow
> a nice implementation for GIL-releasing and non-GIL-releasing C++
> calls in caller.hpp, but the problem with the ResultConverter being
> called from invoke.hpp still remains.  If there was a base class for
> all result converters, it could perhaps acquire the GIL in its
> constructor and release in the destructor, though no such class exists
> (that I can see?), and it would be yet another mutex
> acquisition/release.

This is all very complicated sounding, and I can't tell whether it's
still an issue in light of the guard object idea, and I have so much
to catch up on I don't think I have time to analyze.  If there's
still an issue here, could you boil it down a bit?

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