Gabriel Becedillas wrote:
> Michael Hudson wrote:
>> Gabriel Becedillas <[EMAIL PROTECTED]> writes:
>>
>>
>>> Hi,
>>> At the company I work for, we've embedded Python in C++ application
>>> we develop. Since our upgrade to Python 2.4.2 from 2.4.1 we started
>>> hitting Py_FatalError("Invalid thread state for this thread") when
>>> using debug builds.
>>> We use both multiple interpreters and thread states.
>>
>>
>> I know you've said this to me in email already, but I have no idea how
>> you managed to get this to work with 2.4.1 :)
>>
>>
>>> I think the problem is that PyThreadState_Delete (which is used when I
>>> destroy thread states and interpreters) is not making the same clean up
>>> that PyThreadState_DeleteCurrent is doing (which is used when we
>>> create threads from python code). If a thread id is reused (obviously
>>> not between two running threads), and the previous thread used
>>> PyThreadState_Delete instead of PyThreadState_DeleteCurrent, then an
>>> old and invalid value for autoTLSkey is still stored, and that is
>>> triggering the Py_FatalError when a call to PyThreadState_Swap is made.
>>> If I add this code at the end of PyThreadState_Delete:
>>>
>>> if (autoTLSkey && PyThread_get_key_value(autoTLSkey) == tstate)
>>> PyThread_delete_key_value(autoTLSkey);
>>>
>>> then everything works fine.
>>
>>
>> I think I begin to understand the problem... but this is still
>> fragile: it relies on the fact that you delete a thread state from the
>> OS level thread that created it, but while a thread belonging to a
>> different *interpreter state* has the GIL (or at least: the
>> interpreter state of the thread state being deleted *doesn't* hold the
>> GIL). Can you guarantee that?
>
> Mmm... it doesn't have to do with a race condition or something. Let me
> try to show you with an example (keep in mind that this relies on the
> fact that at some moment the operating system gives you a thread with
> the same id of another thread that allready finished executing):
>
> // Initialize python.
> Py_Initialize();
> PyEval_InitThreads();
> PyThreadState* p_main = PyThreadState_Swap(NULL);
> PyEval_ReleaseLock();
>
> /*
> Asume this block is executed in a separate thread, and that has been
> asigned thread id 10.
> It doesn't matter what the main thread (the thread that initialized
> Python) is doing. Lets assume its waiting for this one to finish.
> */
> {
> PyEval_AcquireLock();
> PyThreadState_Swap(p_main);
> PyThreadState* p_child = PyThreadState_New(p_main->interp);
>
> PyThreadState_Swap(p_child);
> PyRun_SimpleString("print 'mi vieja mula ya no es lo que era'");
>
> PyThreadState_Clear(p_child);
> PyThreadState_Swap(NULL);
> PyThreadState_Delete(p_child);
> PyEval_ReleaseLock();
> // The os level thread exits here.
> }
> /*
> When this thread state gets deleted (using PyThreadState_Delete), the
> autoTLSkey stored for thread id number 10 is not removed, because
> PyThreadState_Delete doesn't have the necesary cleanup code (check
> pystate.c):
>
> if (autoTLSkey && PyThread_get_key_value(autoTLSkey) == tstate)
> PyThread_delete_key_value(autoTLSkey);
>
> PyThreadState_DeleteCurrent behaves correctly because it does have this
> code.
> I think that this code should be added to tstate_delete_common (I've
> done this and everything worked just fine).
> If a new thread executes the same code, and that thread is assigned the
> same thread id, you get the Py_FatalError I'm talking about.
> A workaround for this would be to use PyThreadState_DeleteCurrent
> instead of PyThreadState_Delete.
>
> If you use the following code instead of the one above, the you have no
> way out to the Py_FatalError:
> */
> {
> PyEval_AcquireLock();
> PyThreadState* ret = Py_NewInterpreter();
>
> PyRun_SimpleString("print 'mi vieja mula ya no es lo que era'");
>
> Py_EndInterpreter(ret);
> PyThreadState_Swap(NULL);
> PyEval_ReleaseLock();
> }
> /*
> When this interpreter gets deleted (using Py_EndInterpreter), its thread
> state gets deleted using PyThreadState_Delete, and you are back to the
> beginning of the problem.
> */
>
> I hope this helps to clarify the problem.
> Thanks a lot and have a nice day.
>
>> It seems to me that to be robust, you need a way of saying "delete the
>> thread local value of autoTLSKey from thread $FOO". But this is all
>> very confusing...
>>
>>
>>> Could you please confirm if this is a bug ?
>>
>>
>> Yes, I think it's a bug.
>>
>> Cheers,
>> mwh
>>
>
>
Can anybody tell me if the patch I suggested is ok ?
That will be to add the following code at the end of PyThreadState_Delete:
if (autoTLSkey && PyThread_get_key_value(autoTLSkey) == tstate)
PyThread_delete_key_value(autoTLSkey);
Thank you very much.
--
Gabriel Becedillas
Developer
CORE SECURITY TECHNOLOGIES
Florida 141 - 2º cuerpo - 7º piso
C1005AAC Buenos Aires - Argentina
Tel/Fax: (54 11) 5032-CORE (2673)
http://www.corest.com
_______________________________________________
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
