[C++-sig] Make threading support official

[troy d. straszheim]

Niall Douglas wrote:
> On 10 Apr 2010 at 21:11, troy d. straszheim wrote:
> 
>> I see some special handling in invoke.hpp for 
>> boost::python::objects::detail::py_iter_, maybe that has something to do 
>> with it.  If one did lock/unlock where I suggest above, one wouldn't 
>> have the necessary c++ type information to do such handling.   Google 
>> doesn't turn up much on this case...   hints?
> 
> Yes, I do remember that iterators caught me when I was developing 
> that patch. I remember being stuck for three or four days tracking 
> down the bug and getting very frustrated with the metaprogramming 
> poverty in the MSVC debugger of the time. Not only that, but I also 
> wanted the future ability to specialise a type to set rules for 
> locking or unlocking.

I don't grok it just yet.  The code prevents locking around any call to 
a function that returns a py_iter_, and the specialization mechanism 
you've built would allow you to prevent locking around functions that 
return other types, but that's all it can do.  Do you recall why you 
needed to avoid locking here?  My guess would be that this py_iter_ 
calls back into python, the same situation as with overrides, below.
Are there other cases where you'd want special handling?  What if I wrap 
a function that takes a boost::python::object (ie a call back to python 
could happen at any time)?

>>>> Would you also need to lock in e.g. object_protocol.cpp:
>>>>
>>>> void setattr(object const& target, object const& key, object const& value)
>>>> {
>>>>      if (PyObject_SetAttr(target.ptr(), key.ptr(), value.ptr()) == -1)
>>>>          throw_error_already_set();
>>>> }
>>> Maybe I am missing your point, but surely all accesses to Python must 
>>> hold the GIL first, not least because the GIL also specifies the 
>>> current interpreter to use? (I know that you can get away with some 
>>> calls, but relying on this seems chardly xprudent).
>> Take function new_class(...) in src/object.cpp:  this is called during 
>> BOOST_PYTHON_MODULE(), and invoke.hpp doesn't know about it, therefore 
>> nothing would be locked.
>> [paste]
>> During module import that is:
>>
>> BOOST_PYTHON_MODULE(m)
>> {
>>    class_<T>("T");  // <- here
>> }
>> [paste]
>> Maybe this is a more practical example:
>>
>> void set_foo_attr(object& obj, object& what)
>> {
>>     api::setattr(obj, "foo", what); // leave cpp in here, no unlock
>> }
>>
>> BOOST_PYTHON_MODULE(m)
>> {
>>     def("set_foo_attr", &set_foo_attr);
>> }
>>
>>  >>> class X: pass
>>  >>> x = X()
>>  >>> set_foo_attr(x, ['something'])
>>  >>> print x.foo
>> ['something']
>>
>>
>> Here, enter/exitCPP would be called by invoke.hpp when set_foo_attr 
>> fires,   but not when api::setattr calls PyObject_SetAttrString.
> 
> Ah I see your point now.
> 
> TnFOX's BPL patch made the assumption that when python calls into C++ 
> land that it was going to be executing C++ stuff rather than 
> interfacing with C++ representations of python stuff. This was a very 
> reasonable assumption to make when you are simply providing python 
> bindings for a large C++ library, so on the point of entering C++ 
> land it unlocks the GIL and sets the current python interpreter to 
> null. For that thread, any attempt to do *anything* with python 
> henceforth guarantees an exception.

Hrm.   If I wrap a C++ base class, so that one can inherit from it in 
python, then you've got exactly this situation.  This is a common 
pattern in our application, a C++ framework, configured and run by a 
python script, which can take C++ or python plugins.
So the user runs a python script, which creates an application instance, 
and then tells it to load several modules, some of which are pure c++, 
some are python-inheriting-from-c++.   The python plugins operate on 
objects which are, you guessed it, C++ objects wrapped in python.   So 
you're constantly bouncing back and forth across the language barrier. 
Ah, now I see you come to this in your next paragraph:

> Now if that C++ thread happens to want to do some work in Python, it 
> must hold a RAII instance of FXPython_CtxHold around the relevant 
> code. FXPython_CtxHold simply takes a FXPythonInterp to set as 
> current - this grabs the GIL and sets the current python interpreter.
> 
> I have never been particularly happy with this solution because 
> excess locking is real bad for system latency i.e. when python calls 
> a virtual function the GIL gets unlocked, relocked, a check for a 
> python override is made, unlocked, jump to base class implementation, 
> on exit relock. I personally would use a technique I call "a hanging 
> lock" whereby you wrap a parameter with a thin metaprogramming 
> construct which lets you effectively bind a RAII held lock with a 
> parameter such that a callee takes possession, but I don't know if 
> this is a common technique in Boost and adding it made patching in 
> updates too difficult.

Seems to me that if one wants to claim that boost.python "supports 
multithreading", it needs to support this case, even if there are 
performance hits.  I'd be interested to learn more about these hanging 
locks...

>>> Hence it may well be that a static signals and slots implementation 
>>> could be more appropriate in this situation. I guess I wouldn't know 
>>> until I run benchmarks. Your thoughts?
>> Thanks for the discussion.  I'm now thinking that the handling of these 
>> enter/exit "signals" emitted by boost.python shouldn't be coupled to 
>> boost::signals or anything else.  Seems cleaner and easier to provide an 
>> interface behind which one could put a simple lock/unlocker or something 
>> more complicated involving boost::signals if desired.
> 
> I was maybe thinking of doing it properly with a boost::staticsignals 
> library.
> 

Ah,  I think I interpreted "static signals" to mean boost::signals (not 
the threadsafe boost::signals2).  Anyhow, I think the user should just 
supply something like

struct mylocker
{
   void enterCPP();
   void exitCPP();
};

boost.python would call those at the appropriate times and the user 
could put whatever they wanted inside those functions.   I mentioned a 
tracing facility:  maybe this interface would be more elaborate, 
depending on where inside boost.python these calls originate...  we'll 
see...

>> [snip]
>>> In TnFOX I have a metaprogramming construct which assembles 
> inline a 
>>> jump table of specialisations of classes between which at run 
> time 
>>> can be dynamically chosen. Fairly amazingly, all major compilers 
>>> correctly elide table entries which cannot be chosen such that 
> they 
>>> will remove the choice logic entirely if there is just one 
> possible 
>>> choice, or the whole construct if there are none. This particular 
> 
>>> construct is really useful in BPL actually because it lets you 
> fake 
>>> stuff like setting at runtime arbitrary python code as a C (not 
> C++) 
>>> API based sort function.
>> Could you point me at the code?
> 
> Sure.
> 
> The general TnFOX docs are at http://tnfox.sourceforge.net/TnFOX-
> svn/html/ where the header file FXGenericTools.h 
> (http://tnfox.sourceforge.net/TnFOX-
> svn/html/_f_x_generic_tools_8h.html) contains most of the 
> metaprogramming library. In this file there is a template called 
> FX::Generic::TL::dynamicAt< typelist, instance > 
> (http://tnfox.sourceforge.net/TnFOX-
> svn/html/struct_f_x_1_1_generic_1_1_t_l_1_1dynamic_at.html). You can 
> see the source of FXGenericTools.h at 
> http://tnfox.sourceforge.net/TnFOX-svn/html/_f_x_generic_tools_8h-
> source.html with dynamicAt<> being at around line 2226. As you'll 
> note, dynamicAt<> statically assembles a sixteen entry array of 
> runtime generated code points into read only space, then it jumps 
> into it by array indexation. I have to hand full kudos to the 
> compiler guys such that the compiler knows the right thing to do if 
> the array index is fixed.
> 
> The example I refered to of faking arbitrary python code is 
> documented at  .
> 
> It may help you when you are reading the code to know that 
> FX::Generic::TL::instantiateH<> is a horizontal instantiator of a 
> typelist i.e. TL::instantiateH<TL::create<A, B, C>::value, Holder> 
> will make a class instantiateH<> : public Holder<A>, public 
> Holder<B>, public Holder<C>.
> 
> If you have any questions then please do let me know - I appreciate 
> that yet another metaprogramming library can hurt the head. 

Yeah, ouch.  :)  I'll come back to this...

> With 
> regard to implementing support for all this, could you clarify your 
> planned timetable with respect to your branch of Boost?

Well adding enterCPP() | exitCPP() code to my branch is easy.  It would 
go in this file:

http://gitorious.org/~straszheim/boost/straszheims-python/blobs/master/include/boost/python/detail/caller.hpp

which has replaced all of the preprocessor stuff in detail/invoke.hpp. 
Lines 258 and 265 are where boost::fusion actually calls the registered 
C++ functions;  you could just (un)lock around the call to go() at line 
277.   Then we'd have to figure out what to do about this py_iter_ 
stuff.  Then, AFAICT we'd have in my branch exactly what your patches do 
to the boost.python from svn.

Note: again, that code doesn't compile with MSVC, and I don't know 
enough about the compiler to read its mind and restructure things such 
that it doesn't choke...   Is MSVC support a prerequisite for you to be 
interested in all this?

-t