[Python-Dev] threadsafe patch for asynchat

2006-02-06 Thread Mark Edgington
Does anyone have any comments about applying the following patch to 
asynchat?  It should not affect the behavior of the module in any way 
for those who do not want to use the feature provided by the patch.  The 
point of the patch is to make it easy to use asynchat in a multithreaded 
application.  Maybe I am missing something, and the patch really doesn't 
make it threadsafe?  Any comments would be appreciated.  Also, if it 
looks good to everyone, feel free to use it.

---BEGIN PATCH--
--- asynchat.pyFri Oct 15 03:03:16 2004
+++ asynchat.py.newSun Feb 05 22:05:42 2006
@@ -59,10 +59,11 @@
 ac_in_buffer_size   = 4096
 ac_out_buffer_size  = 4096
 
-def __init__ (self, conn=None):
+def __init__ (self, conn=None, running_in_thread=False):
 self.ac_in_buffer = ''
 self.ac_out_buffer = ''
 self.producer_fifo = fifo()
+self.running_in_thread = runnning_in_thread
 asyncore.dispatcher.__init__ (self, conn)
 
 def collect_incoming_data(self, data):
@@ -157,7 +158,9 @@
 
 def push (self, data):
 self.producer_fifo.push (simple_producer (data))
-self.initiate_send()
+# only initiate a send if not running in a threaded 
environment, since
+# initiate_send() is not threadsafe.
+if not self.running_in_thread: self.initiate_send()
 
 def push_with_producer (self, producer):
 self.producer_fifo.push (producer)
---END PATCH--

-Mark

___
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


Re: [Python-Dev] threadsafe patch for asynchat

2006-02-08 Thread 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() )

___
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