[pyOpenSSL] Python core dumps when using pyOpenSSL 0.8.0 and threads
Jean-Paul Calderone
exarkun at divmod.com
Wed Apr 1 22:26:43 CEST 2009
On Wed, 25 Mar 2009 00:24:29 +0100, "M.-A. Lemburg" <mal at egenix.com> wrote:
>On 2009-03-22 17:32, Jean-Paul Calderone wrote:
>> Hello all,
>>
>> I just pushed a branch to launchpad which may address the 0.8 thread-related
>> crashes.
>>
>> The bug, along with a reproduction script, is described here:
>>
>> https://bugs.launchpad.net/pyopenssl/+bug/344815
>>
>> The branch, along with checkout instructions, which fixes the problem is here:
>>
>> https://code.launchpad.net/~exarkun/pyopenssl/thread-crash
>>
>> If you're not in to bzr, then you can get just the revision which includes
>> the fix here:
>>
>> http://bazaar.launchpad.net/~exarkun/pyopenssl/thread-crash/revision/98
>
Hi Marc,
Thanks for looking into this. Sorry for the delayed response, PyCon has
been keeping me busy.
>This doesn't appear to cover all cases, esp. not if you have an
>application that uses recursion in callbacks.
>
>The patch causes an already existing thread state to get overwritten
>by another one. If the application starts returning from a deeper
>nesting, this will cause a wrong thread state to get restored (actually,
>the last one will get restored multiple times).
I think this will work out okay. The thread state is actually a TLS object
anyway. Any given thread can only have one thread state object. So
PyEval_SaveThread will always return the same PyInterpreterState*. At
least, I think this is how it happens.
Going into a bit more detail...
Say someone calls Connection.send. MY_BEGIN_ALLOW_THREADS is invoked and
PyEval_SaveThread swaps NULL in as the new PyInterpreterState and gives
back the existing PyInterpreterState (call it S1). This gets saved with
pyOpenSSL's _pyOpenSSL_tstate_key. Any key there already is deleted.
Now say one of the pyOpenSSL global callbacks gets invoked. It starts by
using MY_END_ALLOW_THREADS. This loads S1 from _pyOpenSSL_tstate_key and
passes it to PyEval_RestoreThread which swaps S1 in as the new
PyInterpreterState. Then it calls into some arbitrary Python code.
Now say this arbitrary Python code calls another Connection's send
method. MY_BEGIN_ALLOW_THREADS is invoked again. This deletes the
current value of _pyOpenSSL_tstate_key and invokes PyEval_SaveThread
again which swaps in NULL as the new PyInterpreterState and then saves
the existing PyInterpreterState in _pyOpenSSL_tstate_key. Since the
previous step saved S1 as the PyInterpreterState, this is S1 again.
Eventually the send finishes, MY_END_ALLOW_THREADS is invoked. This
the value of _pyOpenSSL_tstate_key which is S1, as described in the
previous step. S1 is passed to PyEval_RestoreThread and then the send
call is done.
Now control returns to the global callback code which finishes up and
gets ready to return back to OpenSSL. It does this by invoking
MY_BEGIN_ALLOW_THREADS which clears _pyOpenSSL_tstate_key and then saves
the result of PyEval_SaveThread (S1) in it. Then control returns to
OpenSSL.
Now the very top Connection.send finishes and the implementation of that
method in pyOpenSSL gets ready to return to the calling Python code. It
invokes MY_END_ALLOW_THREADS which loads S1 from _pyOpenSSL_tstate_key and
passes it to PyEval_RestoreThread. Then it returns to the calling Python
code.
I wasn't actually very confident that the patch was correct, but now that
I've actually written all that up and traced through the various levels of
thread control APIs in CPython, I'm relatively confident that the patch is
doing the right thing. I also learned that there are some APIs in CPython
which do much of what pyOpenSSL is doing. They aren't available in Python
2.3 though, so I'm not quite ready to switch over to them.
How does all this sound?
Jean-Paul
More information about the pyopenssl-users
mailing list