Tim Peters <[EMAIL PROTECTED]> 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 _______________________________________________ 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
