Re: [pygccxml-development] Feature request: Creating thread safe methods

[Matthias B. <ba...@ir...>]

Roman Yakovenko wrote:
> May be we should add to boost::python library simple guard:
> struct gilguard{
>    gilguard(){ PyGILState_Ensure() ; }
>    ~gilguard(){ PyGILState_Release(); }
> };
> 
> Thus the generated code will be able to use the guard and it will be 
> exception neutral.

This is a good idea for the case when the entire function/method has to 
be surrounded by the above calls.

But in the case of our wrapper methods it would be more efficient to do 
the original function call outside the locking to prevent other threads 
from being blocked.
Currently the wrapper code for virtual member functions looks somewhat 
like this:

virtual void foo(){
   if( bp::override func_foo = this->get_override( "foo" ) )
      func_foo( );
   else
      Foo::foo( );
}

Surrounding the entire function body with the Ensure() and Release() 
calls would work but I think it's too conservative. In the case when the 
original Foo:foo() function is invoked the interpreter lock is not 
necessary anymore and it would just prevent other threads from being 
able to execute Python code.
So I think the wrapper code should rather look like this:

virtual void foo(){
   PyGILState_STATE gstate;
   gstate = PyGILState_Ensure();

   bp::override func_foo = this->get_override( "foo" );
   if( func_foo )
   {
      func_foo( );
      // at this point the user might want to add code that
      // checks for exceptions (or does the above function
      // call already raise a C++ exception? then there
      // should be a try...catch block)
   }

   PyGILState_Release(gstate);

   if (!func_foo)
      Foo::foo( );
}

>> However, this code is generated by pyplusplus and currently I don't see
>> a way to inject the extra code.
> 
> Now can you see the problem with virtual functions and arguments policies?
> They don't work well together :-(. The main problem is that function
> signature is changed.

Well, yes, but in this case we are actually dealing with two wrapper 
methods, aren't we? One for the case when the call is made from C++ and 
one for the case when the call is made from Python.
So far, my existing arg policies are dealing with the case when the call 
is made from Python and goes down to C++. Hm, maybe the other direction 
just needs a separate set of "arg policy" classes....?

So far, the virtual methods in the Maya SDK are always called from C++, 
so I would only need one direction. But I have only considered a few 
classes and methods yet, there might still be methods where the derived 
class wants to invoke the inherited method...

>> Maybe this flag could even have three states so
>> that in addition to True and False there's the option to use a global
>> default value. This means the user could simply set the default value to
>> specify if he wants to create thread-safe bindings or not.
> 
> mb.calldefs().thread_safe = True

Well, ok. I just thought it would be more efficient and spare one query, 
but the above is fine with me.

>> Another question is if pyplusplus should ever add the locking functions
>> to non-virtual methods. In principle, non-virtual methods are always
>> thread-safe unless the method invokes Python code internally (which is
>> probably a really rare case).
> 
> I don't know. Also I prefer to develop py++ having real use cases.
> So the answer is next: if you don't have such use case we will not
> write the code that will treat it.

ok. As I already said, I think it's really a rare case where someone 
would need this. Usually, the methods will be virtual so that you can 
override the methods and implement them in Python.

>> Then there's another issue related to thread-safety. Locking the Python
>> interpreter is one thing, but you also have to catch any exception that
>> is raised in the Python code and take some appropriate actions (like
>> reporting the exception and, in my case, returning an appropriate error
>> status code to Maya).
> 
> Will exception neutral code will solve this problem? 

What do you mean by this?

> If not, I think py++
> should have functionality, that will allow user to add/modify/rewrite
> code generated
> by py++. If this functionality will be added, we could implement
> "arguments policy"
> functionality out side of py++.

If you mean that the actual policy class is outside py++ (but of course, 
the framework for this to work is inside py++), then I agree.... :)

> Every overridable virtual member function looks like:
> 
> 0 virtual int pure_virtual( int x ){
> 1
> 2    bp::override func_pure_virtual = this->get_override( "pure_virtual" );
> 3
> 4 return func_pure_virtual( x );
> 5
> 6 }
> 
> It seems, that user needs to add code to 1, 3, 5 and to be able to 
> modify 2,4. Am I right?

In principle, yes. Of course, as I wasn't dealing with virtual methods 
yet I have no distinction between 1 and 3 (as line 2 doesn't appear in 
the code generated by the existing arg policy mechanism), so both of 
them would be "pre-call" whereas 5 is "post-call". Additionally, there 
should be a way to allocate local variables (this has to be done via an 
API call so that the variable name can be adjusted if necessary to 
prevent name clashes).

- Matthias -