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

[Vladimir Vukicevic]

On Sun, 2003-07-13 at 14:40, David Abrahams wrote:
> Vladimir Vukicevic <vladimir at pobox.com> writes:
> 
> > Sorry 'bout that, I was in a hurry when I wrote that and didn't manage
> > to explain concisely.
> >
> > Essentially what I think needs to happen is the following:
> >
> > 1. The GIL needs to be acquired at every point that python calls into
> > boost::python glue code.  
> 
> I don't think so!
> 
> When Python calls boost::python it *always* has the GIL already.  It
> looks like you have way too many detail::gil_lock instances in your
> patch.  GUI acquisition needs to be done when C++ code calls back into
> python.

Hmm.. you're right.  There were a few problems that I had going on that
made me put in a lot of gil_lock's, more so than what are needed.  I'll
have to go through and examine again exactly where they need to go.

> > (Any such acquisition should be optional, such that extensions that
> > don't care about threads don't have to deal with the overhead.)
> 
> Which is why I don't understand the need for call_with_gil<>.  What's
> wrong with asking the user to create a local gil_lock of his own?  He
> might need one anyway, since he may need to use the
> Boost.Python/Python API himself.

Sure, sounds good.

> > 2. The GIL needs to be released (threads "allowed") around every call to
> > a native C++ function wrapped by boost::python.  (This function should,
> > if it needs to call Python or boost::python API calls, acquire the GIL
> > itself.)
> 
> No way.  That incurs way too much overhead for small wrapped
> functions.  Releasing the GIL when we call a wrapped C++ function
> needs to be optional (possibly globally settable).

Are you thinking settable at runtime or at compile time?  If at compile
time, then defining BOOST_PYTHON_NO_THREADING causes
gil_lock/thread_block to just be dummy stubs.  I don't see how you'd do
it for some methods and not for others, without adding extra logic to
the path that leads to invoke() -- which I guess may be the right thing
to do for a fully general solution.

> In any case, even if it's not optional you have a problem because
> you're releasing the GIL when calling into functions that accept,
> e.g. python::object parameters.  You haven't patched
> object::object(object const&) to acquire the gil.  You'd need to find
> *every* PyXXXX in the library and make sure the GIL is acquired.  All
> that aqcuiring/releasing could get expensive, and I'm not comfortable
> with that.

Yup, there's still a bunch of places where the locks aren't correct. 
Expensive or not though, I don't see a way around it -- you're either
not thread safe (in which case you go the BOOST_PYTHON_NO_THREADING
route at compile time), or you are -- eventually one could probably
build separate boost_python .so with and without thread safety so that
extensions can choose which version to link/build with.

Thanks for the feedback,
	- Vlad