classification
Title: smtpd.SMTPServer can cause asyncore.loop to enter infinite event loop
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.1, Python 3.2, Python 2.7, Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: giampaolo.rodola Nosy List: cmcginty, ggenellina, giampaolo.rodola, josiah.carlson, r.david.murray
Priority: normal Keywords: patch

Created on 2009-07-28 03:59 by cmcginty, last changed 2010-06-30 17:53 by giampaolo.rodola. This issue is now closed.

Files
File name Uploaded Description Edit
smtp_test.py cmcginty, 2010-06-29 22:20 Script to reproduce reported issue
smtpd.patch giampaolo.rodola, 2010-06-30 17:13
Messages (8)
msg91002 - (view) Author: Casey McGinty (cmcginty) Date: 2009-07-28 03:59
When subclass of smtpd.SMTPServer, it is possible the get asyncore.loop
to enter an infinite loop where the following output is shown:

.....
warning: unhandled write event
warning: unhandled read event
warning: unhandled write event
warning: unhandled read event
warning: unhandled write event
warning: unhandled read event
warning: unhandled write event
warning: unhandled read event
warning: unhandled write event
warning: unhandled read event
warning: unhandled write event
warning: unhandled read event
....

To reproduce:
1) Init SMTPServer class instance inside of Thread class and call
asyncore.loop()
2) Init second SMTPServer class instance from a second Thread class,
binding to the same address and port. The SMTPServer instance will raise
an exception that socket cannot bind to the port in use. The socket
exception must be caught at some level, and the program execution
allowed to continue.
3) First SMTPServer thread will enter infinite event loop.


Analysis:
When the exception is raised in the second SMTPServer instance, the new
instance has already registered itself with the asyncore library (ex:
'asyncore.dispatcher.__init__(self)'). There is no code in the
SMTPServere.__init__ method that catches the exception raised, caused by
the failed bind attempt. After the exception is caught, the first thread
continues to run, but asyncore is in an invalid state because it still
has the registration of the second SMTPServer instance that never completed.

Workaround and Proposed Fix:
A viable workaround seems to be catching the raised exception in
__init__ method of the SMTPServer subclass, and call self.close() before
re-raising the exception:

class MySmtpServer( smtpd.SMTPServer, object ):
   def __init__( self, **kwargs ):
      try:
         super( _SmtpSink, self).__init__(**kwargs)
      except Exception as e:
         self.close()   # cleanup asyncore after failure
         raise

For a long term fix, I would recommend performing the
asyncore.dispatcher.close() method call in the SMTPServer.__init__() method.
msg106073 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-05-19 13:50
Could you provide an actual example code which reproduces this problem?
It's not clear to me how the dispatcher instance can end up in an "invalid state" since handle_error() should automatically remove the invalid dispatcher instance from the socket_map if anything unexpected happens.
If this doesn't happen it might be a problem related with what you've done in your subclass (e.g. you have overridden handle_error and avoided to call close() yourself).

Furthermore the use case you have described it's pretty uncommon as you shouldn't run SMTPServer in a thread in the first place.

If you intend to bind two servers simultaneously you just have to instantiate two STMPServer sub/classes and finally call asyncore.loop().
Both instances will automatically be served by asyncore itself.
msg108867 - (view) Author: Casey McGinty (cmcginty) Date: 2010-06-28 21:00
This is how it gets in an "invalid state". Not sure why you can't look at the code and see the mistake.

"There is no code in the
SMTPServere.__init__ method that catches the exception raised, caused by
the failed bind attempt. After the exception is caught, the first thread
continues to run, but asyncore is in an invalid state because it still
has the registration of the second SMTPServer instance that never completed."

As far as running in a thread. I have a program that needs must start and stop the SMTPServer dynamically. I did this by putting SMTPServer in a thread. Maybe there is another way to do it, but if you are not going to support this, then it should be documented.
msg108888 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-06-29 02:09
If you can provide a short example that reproduces the problem it will be much more likely to get fixed.
msg108955 - (view) Author: Casey McGinty (cmcginty) Date: 2010-06-29 22:20
I attached a simple script that reproduces the report issue. I hope it helps.
msg108974 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-06-30 11:38
Although the use case is pretty uncommon and somewhat twisted (take a look at Lib/test/test_ftplib.py for a nicer approach on wrapping asyncore.loop() in a thread) it is true that if SMTPServer class raise an exception at instantiation time, some garbage remains in asyncore.

To replicate this problem there's no need to involve threads:

>>> import asyncore, smtpd
>>> s = smtpd.SMTPServer(('127.0.0.1', "xxx"),None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.5/smtpd.py", line 280, in __init__
    self.bind(localaddr)
  File "/usr/local/lib/python2.5/asyncore.py", line 303, in bind
    return self.socket.bind(addr)
  File "<string>", line 1, in bind
TypeError: an integer is required
>>> asyncore.socket_map
{3: <smtpd.SMTPServer ('127.0.0.1', 'xxx') at 0xb783528c>}
>>> 

I think it's ok for SMTPServer.__init__ to cleanup asyncore and finally raise the exception, as you suggested in the first place.
I'll provide a patch later today.
msg108998 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-06-30 17:13
Patch in attachment.
msg109000 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-06-30 17:53
Fixed in r82404, r82406, r82407 and r82408.
History
Date User Action Args
2010-06-30 17:53:45giampaolo.rodolasetstatus: open -> closed
resolution: fixed
messages: + msg109000

stage: test needed ->
2010-06-30 17:13:01giampaolo.rodolasetfiles: + smtpd.patch
keywords: + patch
messages: + msg108998
2010-06-30 12:47:19giampaolo.rodolasetnosy: + josiah.carlson
2010-06-30 11:38:58giampaolo.rodolasetassignee: giampaolo.rodola
messages: + msg108974
versions: + Python 3.1, Python 2.7, Python 3.2
2010-06-29 22:20:59cmcgintysetfiles: + smtp_test.py

messages: + msg108955
2010-06-29 02:09:14r.david.murraysetnosy: + r.david.murray
messages: + msg108888

type: behavior
stage: test needed
2010-06-28 21:00:57cmcgintysetmessages: + msg108867
2010-05-19 13:50:50giampaolo.rodolasetmessages: + msg106073
2010-05-19 13:50:11giampaolo.rodolasetmessages: - msg106072
2010-05-19 13:44:03giampaolo.rodolasetmessages: + msg106072
2010-03-14 22:28:46giampaolo.rodolasetnosy: + giampaolo.rodola
2009-08-03 07:59:23ggenellinasetnosy: + ggenellina
2009-07-28 03:59:57cmcgintycreate