[Python-Dev] pystate.c changes for Python 2.4.2 (original) (raw)

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 PyThreadStateDelete:

if (autoTLSkey && PyThreadgetkeyvalue(autoTLSkey) == tstate) PyThreaddeletekeyvalue(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 WITHTHREAD defined. So you need to wrap that inside an "#ifdef WITHTHREAD" block; or add it similarly to tstatedeletecommon(), and remove it from PyThreadStateDeleteCurrent().

The primary reason people find this so confusing is because it is :-). In the dim mists of history, neither multiple interpreters nor PyThreadStateDeleteCurrent() 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 PyThreadStateDeleteCurrent() was introduced to repair those. As a result, it so happens that core Python never uses the original PyThreadStateDelete() anymore, except when PyNewInterpreter() 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 PyNewInterpreter() or PyThreadStateDelete(), 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 PyThreadStateDelete() 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