[C++-sig] Re: Patch to add thread support to invoke function.

[David Abrahams]

"Lijun Qin" <qinlj at solidshare.com> writes:

> Hi,
>
> I think a better solution is to give the user choice, I mean use a flag
> which can be passed when calling 'def', such as
> .def('f', f, enableThreadLock)

If you read the links in my previous posting you'll see that I think
that's appropriate for some wrapping jobs; for others (such as
wrapping Qt calls) it seems that calls are so likely to result in a
call back into Python which *must* unconditionally acquire the GIL
that people just want to unblock everywhere.

So far, however, I'm not sure how to do that without incurring ODR
problems, so we may have to go with your idea.  Of course, that "flag"
should be rolled into the available functionality of CallPolicies.

> Currently I'm using boost.python to wrap WTL, and it has hundreds of
> functions which are simple SendMessage wrappers, and many of them (I
> think half of them, almost all 'write' operation) can cause callback
> into the python code again, use the notify message such as
> WM_COMMAND, WM_NOTIFY. So write wrapper for every one of them is not
> acceptable.  The callback will be implemented use call_method<>
> normally, but the thread acquiring will not be done is call_method,
                                                      ^^
                                                     "in"
> it'll normally be done earlier in the very point the control
> transferred from the 'outside world' to the python extension module,
> in the Windows platform, this will normally at the beginning of COM
> interface method, Window Procedure, or DLL exported function (this
> is rare I think).  

When you say "thread acquiring" do you mean the reacquisition of the
Python GIL and thread state?  Why would you do it any earlier than in,
or just prior to, call_method<>?

> If we can implement the 'enableThreadLock' flag, but make it a
> 'false' default, it'll be harmless to add this thread support into
> boost.python code, but when needed, it'll be very convenience to
> have them.

I think that's a great approach.  Maybe you should extend your prior
CallPolicies patch to do it.

> I do agree that there should be a try catch block here, like:
>
> Py_UNBLOCK_THREADS
> try {
>     result = f(...);
> } catch(...)
>     Py_BLOCK_THREADS
>     throw;
> }
> Py_BLOCK_THREADS
> ...

I wasn't quite suggesting that.  First of all Py_UNBLOCK_THREADS needs
a PyThreadState* variable.  Secondly, using an object whose destructor
unblocks makes it much easier to select thread blocking or not at
compile time. Something like:

   struct thread_enable
   {
        thread_enable()
          : m_thread_state(PyEval_SaveThread())
        {}

        ~thread_enable()
        {
            PyEval_RestoreThread(m_thread_state);
        }
    private:
        PyThreadState* m_thread_state;
   };

We'll just declare one of these on the stack when the thread_enable
flag is used, and declare an unused int when it's not.

A similar, but inverted thread state acquisition object should be
used around call_method<> invocations.  My biggest question is: does
it need a thread state, or should it just use PyEval_AcquireLock(),
and if it *does* need a thread state, where will it come from?

> In my case, because SendMessage and COM method will never throw an
> exception, so I ignored them in the last patch, for a complete solution,
> it'll be better to do this.


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