[Python-Dev] pystate.c changes for Python 2.4.2

Michael Hudson mwh at python.net
Fri Jan 20 10:48:00 CET 2006


Tim Peters <tim.peters at gmail.com> writes:

> [Gabriel Becedillas]
>> 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);
>
> It needs a little work, but I think it should be fine, and people tend
> to listen to me on issues like this ;-)

Oh good, I've been meaning to scrape together the brain cells to think
about this but PyPy is pulping them as fast as I find them...

> The reason it can't work as-is is shallow:  autoTLSkey doesn't exist
> unless Python is compiled with WITH_THREAD defined.  So you need to
> wrap that inside an "#ifdef WITH_THREAD" block; or add it similarly to
> tstate_delete_common(), and remove it from
> PyThreadState_DeleteCurrent().
>
> The primary reason people find this so confusing is because it is :-).
>  In the dim mists of history, neither multiple interpreters nor
> PyThreadState_DeleteCurrent() existed.  While multiple interpreters do
> exist now, they're poorly understood, and the PEP that introduced the
> GIL state API explicitly disavowed any responsibility for the new API
> working at all with multiple interpreters:
>
>     http://www.python.org/peps/pep-0311.html
>
>     This proposal identifies a solution for extension authors with
>     complex multi-threaded requirements, but that only require a
>     single "PyInterpreterState".  There is no attempt to cater for
>     extensions that require multiple interpreter states.  At the time
>     of writing, no extension has been identified that requires
>     multiple PyInterpreterStates, and indeed it is not clear if that
>     facility works correctly in Python itself.
>
> In a parallel development, thread races during Python shutdown were
> discovered that could lead to segfaults, and
> PyThreadState_DeleteCurrent() was introduced to repair those.  As a
> result, it so happens that core Python never uses the original
> PyThreadState_Delete() anymore, except when Py_NewInterpreter() has to
> throw away the brand new thread state it created because it turns out
> it can't create a new interpreter.

Um, PyThreadState_Delete() is called from zapthreads() is called from
PyInterpreterState_Delete() is called from Py_Finalize()... so I don't
entirely believe you here :)

> Since core Python never calls Py_NewInterpreter() or
> PyThreadState_Delete(), you're in an intersection of areas no current
> Python developer even thinks about, let alone tests.  But by the same
> token, _because_ it's unused, there's nothing you can do to
> PyThreadState_Delete() that can hurt core Python either.  That's why
> people should be more encouraging here ;-)  It certainly makes sense
> to me that if a thread state is going away, any record of it in the
> auto-GIL-state machinery must also go away.

I guess if the patch fixes the original problem, it should go in -- my
uneasiness stems not from worrying that it might do harm, but rather
from the fact that I think it's incomplete.  Maybe I just haven't
thought it through enough.

Also, every time I look at pystate.c, I think of things that could be
done differently or better -- for example, I think it would be sane
and possibly even sensible to only call PyThread_set_key_value() in
_PyGILState_NoteThreadState() if "tstate->interp ==
autoInterpreterState".

>> Thank you very much.
>
> Thank you!  If you put a patch on SourceForge, I'll probably be happy
> to check it in.

That would get me off the "svn blame" hook :)

Cheers,
mwh

-- 
  My first thought was someone should offer Mr. Bush elocution
  lessons.  But he'd probably say he knew how to work them chairs
  already.                            -- Internet Oracularity #1294-01


More information about the Python-Dev mailing list