[Python-Dev] threadsafe patch for asynchat

[Mark Edgington]

Martin v. Löwis wrote:
 > That patch looks wrong. What does it mean to "run in a thread"?
 > All code runs in a thread, all the time: sometime, that thread
 > is the main thread.
 >
 > Furthermore, I can't see any presumed thread-unsafety in asynchat.

Ok, perhaps the notation could be improved, but the idea of the 
semaphore in the patch is "Does it run inside of a multithreaded 
environment, and could its push() functions be called from a different 
thread?"

I have verified that there is a problem with running it in such an 
environment.  My results are more or less identical to those described 
in the following thread:
http://mail.python.org/pipermail/medusa-dev/1999/000431.html
(see also the reply message to this one regarding the solution -- if you 
look at the Zope source, Zope deals with the problem in the way I am 
suggesting asynchat be patched)

It seems that somehow in line 271 (python 2.4) of asynchat.py, 
producer_fifo.list is not empty, and thus popleft() is executed. 
However, popleft() finds the deque empty.  This means that somehow the 
deque (or list -- the bug is identical in python 2.3) is emptied between 
the if() and the popleft(), so perhaps asyncore.loop(), running in a 
different thread from the thread which calls async_chat.push(), empties it.

The problem is typically exhibited when running in a multithreaded 
environment, and when calling the async_chat.push() function many (i.e. 
perhaps tens of thousands) times quickly in a row from a different 
thread.  However, this behavior is avoided by creating a Lock for 
refill_buffer(), so that it cannot be executed simultaneously.  It is 
also avoided by not executing initiate_send() at all (as is done by Zope 
in ZHTTPServer.zhttp_channel).

 > Sure, there is a lot of member variables in asynchat which aren't
 > specifically protected against mutual access from different threads.
 > So you shouldn't be accessing the same async_chat object from multiple
 > threads.

If applying this patch does indeed make it safe to use async_chat.push() 
from other threads, why would it be a bad thing to have?  It seems to 
make the code less cryptic (i.e. I don't need to override base classes 
in order to include code which processes a nonempty Queue object -- I 
simply make a call to the push() function of my instance of async_chat, 
and I'm done).

-Mark

(also, of course push_with_producer() would probably also need the same 
changes that would be made to push() )