PyThreadState_Swap fatal error (was Re: [ANN] --enable-dbgutil: everyone gets a Debug STL!)

[Michael Stahl]

this could benefit from advice of a CPython hacker :)

On 17/04/13 09:33, Stephan Bergmann wrote:
> On 04/15/2013 03:34 PM, Michael Stahl wrote:
>> with commit f14f7a2e4568e3e85a0c8860beebd0376c5a8b51 MSVC builds will
>> link everything against the debug runtimes (MSVCRTD etc.), which enables
>> debug STL and other things.
> [...]
>> the MSVC one is tested on MSVC 2008 only, where "make check" passes
>> successfully; it's possible that i've overlooked something in the
>> makefiles where different MSVC versions use different rules, so it needs
>> somebody to try it out in an --enable-dbgutil build with 2010 and 2012.
> 
> You probably did that "make check" on a build without any bundled Python 
> extensions, right?  In my heavily --enable-ext-* configured build, 
> starting soffice instantly fails with
> 
>> Fatal Python error: Invalid thread state for this thread
>>
>> Current thread 0x00000260:
>>
>> Thread 0x00000260:

after enabling some Python extensions (i was actually unaware that my
MSVC build was without dictionaries since they started defaulting to
disabled...), i can reproduce the problem.

[ the Py_FatalError function calls abort() which then causes your other
assertion/dialog from operator delete (somehow). ]

the "invalid thread state" is apparently caused by the
PyThreadAttach / PyThreadDetach classes in PyUno, which are both RAII
style guard classes.

the PyThreadAttach will create a new PyThreadState with
PyThreadState_New and call PyEval_AcquireThread in its ctor, and
PyEval_ReleaseThread and PyThreadState_Delete in its dtor.

PyThreadDetach does the inverse, calling PyEval_ReleaseThread in the
ctor and PyEval_AcquireThread in the dtor.

now it may happen that a single OS thread does multiple independent
calls into PyUno, for example a call into a Python UNO component, which
calls a C++ UNO component, which in turn calls a Python UNO component.

in this case we get multiple PyThreadState for the same OS thread,
because multiple PyThreadAttach objects are on the C stack (with
PyThreadDetach between them).

this is apparently what the assertion in PyThreadState_Swap that is
triggered here complains about: it checks that the value of
PyGILState_GetThisThreadState(), which essentially is a thread-local
variable holding the last PyThreadState that was used on the current OS
thread, is the same as the PyThreadState that is passed in.

now i think we could ensure that this is the case in PyUno, by storing
the PyThreadState in a thread-local variable in PyThreadAttach so that
one OS thread only uses one PyThreadState.

but i have doubts that this actually makes sense - this PyThreadState
apparently contains things like pending exceptions, and i'm concerned
that if the "inner" Python evaluation would use these that they could
confuse the "outer" Python evaluation when control returns to it.

the other alternative that i see is to simply patch out the check in
PyThreadState_Swap; the _other_ checks in PyEval_*Thread functions and
RAII nature of the guard classes should hopefully prevent ~all of the
potential errors.