On Thu, Jun 07, 2007 at 12:36:25AM +0300, Marius Gedminas wrote: > On Wed, Jun 06, 2007 at 06:04:50PM +0200, Sascha Ottolski wrote: > > I discovered a problem with the maildir implementation, that is > > triggered when trying to use the QueuedDelivery with many (read: more > > than current filedescriptor limit of the zope process) mails at once: > ... > > OSError: [Errno 24] Too many open files: > > '/home/so/sandbox/var/zope/lib/python/lalisio/app/site/../../../../var/zope/var/mqueue/new' ... > > The reason for that to happen is clear to me, however, I'm not sure how > > a fix could look like. > > Did you create an bug in launchpad? I could attach a patch that I think > would fix the problem, but I don't currently have the time and energy to > test myself.
I'm attaching the patch here. I'd be happy to commit it to trunk if
somebody else could test it on various platforms. (I haven't even
tested if it works fine on Linux, other than by running the unit tests.)
Marius Gedminas
--
(mental note: stop installing red hat. everytime i do so, it takes ages to fix
my system again.)
-- from the sig of Martin H�gman
Make QueuedMailDelivery not keep open files around for every message sent
during a transaction.
Patch by Marius Gedminas <[EMAIL PROTECTED]>
Index: tests/test_maildir.py
===================================================================
--- tests/test_maildir.py (revision 75881)
+++ tests/test_maildir.py (working copy)
@@ -19,6 +19,7 @@
import unittest
import stat
import os
+import errno
from zope.interface.verify import verifyObject
@@ -123,7 +124,7 @@
def open(self, filename, flags, mode=0777):
if (flags & os.O_EXCL and flags & os.O_CREAT
and self.access(filename, 0)):
- raise OSError('file already exists')
+ raise OSError(errno.EEXIST, 'file already exists')
if not flags & os.O_CREAT and not self.access(filename, 0):
raise OSError('file not found')
fd = max(self._descriptors.keys() + [2]) + 1
Index: tests/test_delivery.py
===================================================================
--- tests/test_delivery.py (revision 75881)
+++ tests/test_delivery.py (working copy)
@@ -147,18 +147,32 @@
data = ''
commited_messages = [] # this list is shared among all instances
aborted_messages = [] # this one too
+ _closed = False
def write(self, str):
+ if self._closed:
+ raise AssertionError('already closed')
self.data += str
def writelines(self, seq):
+ if self._closed:
+ raise AssertionError('already closed')
self.data += ''.join(seq)
+ def close(self):
+ self._closed = True
+
def commit(self):
+ if not self._closed:
+ raise AssertionError('for this test we want the message explicitly'
+ ' closed before it is committed')
self._commited = True
self.commited_messages.append(self.data)
def abort(self):
+ if not self._closed:
+ raise AssertionError('for this test we want the message explicitly'
+ ' closed before it is committed')
self._aborted = True
self.aborted_messages.append(self.data)
Index: maildir.py
===================================================================
--- maildir.py (revision 75881)
+++ maildir.py (working copy)
@@ -18,6 +18,7 @@
__docformat__ = 'restructuredtext'
import os
+import errno
import socket
import time
import random
@@ -91,7 +92,9 @@
filename = join(subdir_tmp, unique)
try:
fd = os.open(filename, os.O_CREAT|os.O_EXCL|os.O_WRONLY, 0600)
- except OSError:
+ except OSError, e:
+ if e.errno != errno.EEXIST:
+ raise
# File exists
counter += 1
if counter >= 1000:
@@ -115,7 +118,7 @@
self._filename = filename
self._new_filename = new_filename
self._fd = fd
- self._closed = False
+ self._finished = False
self._aborted = False
def write(self, data):
@@ -124,22 +127,28 @@
def writelines(self, lines):
self._fd.writelines(lines)
+ def close(self):
+ self._fd.close()
+
def commit(self):
- if self._closed and self._aborted:
+ if self._aborted:
raise RuntimeError('Cannot commit, message already aborted')
- elif not self._closed:
- self._closed = True
- self._aborted = False
+ elif not self._finished:
+ self._finished = True
self._fd.close()
os.rename(self._filename, self._new_filename)
# NOTE: the same maildir.html says it should be a link, followed by
# unlink. But Win32 does not necessarily have hardlinks!
def abort(self):
- if not self._closed:
- self._closed = True
+ # XXX mgedmin: I think it is dangerous to have an abort() that does
+ # nothing when commit() already succeeded. But the tests currently
+ # test that expectation.
+ if not self._finished:
+ self._finished = True
self._aborted = True
self._fd.close()
os.unlink(self._filename)
- # should there be a __del__ that does abort()?
+ # XXX: should there be a __del__ that calls abort()?
+
Index: interfaces.py
===================================================================
--- interfaces.py (revision 75881)
+++ interfaces.py (working copy)
@@ -243,6 +243,15 @@
any line separators.
"""
+ def close():
+ """Closes the message file.
+
+ No further writes are allowed. You can call ``close()`` before calling
+ ``commit()`` or ``abort()`` to avoid having too many open files.
+
+ Calling ``close()`` more than once is allowed.
+ """
+
def commit():
"""Commits the new message using the `Maildir` protocol.
Index: delivery.py
===================================================================
--- delivery.py (revision 75881)
+++ delivery.py (working copy)
@@ -134,6 +134,7 @@
msg.write('X-Zope-From: %s\n' % fromaddr)
msg.write('X-Zope-To: %s\n' % ", ".join(toaddrs))
msg.write(message)
+ msg.close()
return MailDataManager(msg.commit, onAbort=msg.abort)
signature.asc
Description: Digital signature
_______________________________________________ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
