[Zope3-checkins] SVN: Zope3/trunk/src/zope/app/mail/ Bugfix: fix the race between the os.path.exists check and open(filename, "w")

Marius Gedminas marius at pov.lt
Fri Apr 21 13:26:12 EDT 2006


Log message for revision 67234:
  Bugfix: fix the race between the os.path.exists check and open(filename, "w")
  when two different threads try to create messages in the same Maildir queue.
  
  Happily, os.open(filename, os.O_CREAT|os.O_EXCL|os.O_WRONLY) works in Windows
  too, not just POSIX.
  
  This fixes another part of http://www.zope.org/Collectors/Zope3-dev/590.
  
  

Changed:
  U   Zope3/trunk/src/zope/app/mail/maildir.py
  U   Zope3/trunk/src/zope/app/mail/tests/test_maildir.py

-=-
Modified: Zope3/trunk/src/zope/app/mail/maildir.py
===================================================================
--- Zope3/trunk/src/zope/app/mail/maildir.py	2006-04-21 17:23:38 UTC (rev 67233)
+++ Zope3/trunk/src/zope/app/mail/maildir.py	2006-04-21 17:26:12 UTC (rev 67234)
@@ -82,20 +82,26 @@
         pid = os.getpid()
         host = socket.gethostname()
         counter = 0
-        while 1:
+        while True:
             timestamp = int(time.time())
             unique = '%d.%d.%s' % (timestamp, pid, host)
             filename = join(subdir_tmp, unique)
-            if not os.path.exists(filename):
+            try:
+                fd = os.open(filename, os.O_CREAT|os.O_EXCL|os.O_WRONLY, 0600)
+            except OSError:
+                # File exists
+                counter += 1
+                if counter >= 1000:
+                    raise RuntimeError("Failed to create unique file name"
+                                       " in %s, are we under a DoS attack?"
+                                       % subdir_tmp)
+                # NOTE: maildir.html (see above) says I should sleep for 2
+                #       seconds, not 1
+                time.sleep(1)
+            else:
                 break
-            counter += 1
-            if counter >= 1000:
-                raise RuntimeError("Failed to create unique file name in %s,"
-                                   " are we under a DoS attack?" % subdir_tmp)
-            # NOTE: maildir.html (see above) says I should sleep for 2
-            #       seconds, not 1
-            time.sleep(1)
-        return MaildirMessageWriter(filename, join(subdir_new, unique))
+        return MaildirMessageWriter(os.fdopen(fd, 'w'), filename,
+                                    join(subdir_new, unique))
 
 
 class MaildirMessageWriter(object):
@@ -103,13 +109,10 @@
 
     implements(IMaildirMessageWriter)
 
-    # A hook for unit tests
-    open = open
-
-    def __init__(self, filename, new_filename):
+    def __init__(self, fd, filename, new_filename):
         self._filename = filename
         self._new_filename = new_filename
-        self._fd = self.open(filename, 'w')
+        self._fd = fd
         self._closed = False
         self._aborted = False
 

Modified: Zope3/trunk/src/zope/app/mail/tests/test_maildir.py
===================================================================
--- Zope3/trunk/src/zope/app/mail/tests/test_maildir.py	2006-04-21 17:23:38 UTC (rev 67233)
+++ Zope3/trunk/src/zope/app/mail/tests/test_maildir.py	2006-04-21 17:26:12 UTC (rev 67234)
@@ -15,8 +15,10 @@
 
 $Id$
 """
+
 import unittest
 import stat
+import os
 
 from zope.interface.verify import verifyObject
 
@@ -42,20 +44,20 @@
         self.files = files
         self.dirs = dirs
 
-    _exists_never_fails = False
-
     def join(self, *args):
         return '/'.join(args)
 
     def isdir(self, dir):
         return dir in self.dirs
 
-    def exists(self, p):
-        return self._exists_never_fails or p in self.files
 
 class FakeOsModule(object):
 
     F_OK = 0
+    O_CREAT = os.O_CREAT
+    O_EXCL = os.O_EXCL
+    O_WRONLY = os.O_WRONLY
+
     _stat_mode = {
         '/path/to/maildir': stat.S_IFDIR,
         '/path/to/maildir/new': stat.S_IFDIR,
@@ -84,8 +86,13 @@
     _removed_files = ()
     _renamed_files = ()
 
+    _all_files_exist = False
+
+    def __init__(self):
+        self._descriptors = {}
+
     def access(self, path, mode):
-        return path in self._stat_mode
+        return path in self._stat_mode or self._all_files_exist
 
     def stat(self, path):
         if path in self._stat_mode:
@@ -107,6 +114,37 @@
     def rename(self, old, new):
         self._renamed_files += ((old, new), )
 
+    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')
+        if not flags & os.O_CREAT and not self.access(filename, 0):
+            raise OSError('file not found')
+        fd = max(self._descriptors.keys() + [2]) + 1
+        self._descriptors[fd] = filename, flags, mode
+        return fd
+
+    def fdopen(self, fd, mode='r'):
+        try:
+            filename, flags, permissions = self._descriptors[fd]
+        except KeyError:
+            raise AssertionError('os.fdopen() called with an unknown'
+                                 ' file descriptor')
+        if mode == 'r':
+            assert not flags & os.O_WRONLY
+            assert not flags & os.O_RDWR
+        elif mode == 'w':
+            assert flags & os.O_WRONLY
+            assert not flags & os.O_RDWR
+        elif mode == 'r+':
+            assert not flags & os.O_WRONLY
+            assert flags & os.O_RDWR
+        else:
+            raise AssertionError("don't know how to verify if flags match"
+                                 " mode %r" % mode)
+        return FakeFile(filename, mode)
+
+
 class FakeFile(object):
 
     def __init__(self, filename, mode):
@@ -124,10 +162,7 @@
     def writelines(self, lines):
         self._written += ''.join(lines)
 
-def fake_open(self, filename, mode):
-    return FakeFile(filename, mode)
 
-
 class TestMaildir(unittest.TestCase):
 
     def setUp(self):
@@ -139,15 +174,13 @@
         maildir_module.os = self.fake_os_module = FakeOsModule()
         maildir_module.time = FakeTimeModule()
         maildir_module.socket = FakeSocketModule()
-        maildir_module.MaildirMessageWriter.open = fake_open
 
     def tearDown(self):
         self.maildir_module.os = self.old_os_module
         self.maildir_module.time = self.old_time_module
         self.maildir_module.socket = self.old_socket_module
-        self.maildir_module.MaildirMessageWriter.open = open
         self.fake_os_module._stat_never_fails = False
-        self.fake_os_module.path._exists_never_fails = False
+        self.fake_os_module._all_files_exist = False
 
     def test_factory(self):
         from zope.app.mail.interfaces import IMaildirFactory, IMaildir
@@ -201,7 +234,7 @@
     def test_newMessage_never_loops(self):
         from zope.app.mail.maildir import Maildir
         from zope.app.mail.interfaces import IMaildirMessageWriter
-        self.fake_os_module.path._exists_never_fails = True
+        self.fake_os_module._all_files_exist = True
         m = Maildir('/path/to/maildir')
         self.assertRaises(RuntimeError, m.newMessage)
 
@@ -209,8 +242,8 @@
         from zope.app.mail.maildir import MaildirMessageWriter
         filename1 = '/path/to/maildir/tmp/1234500002.4242.myhostname'
         filename2 = '/path/to/maildir/new/1234500002.4242.myhostname'
-        writer = MaildirMessageWriter(filename1, filename2)
-        # writer._fd should be a FakeFile instance because we stubbed open()
+        fd = FakeFile(filename1, 'w')
+        writer = MaildirMessageWriter(fd, filename1, filename2)
         self.assertEquals(writer._fd._filename, filename1)
         self.assertEquals(writer._fd._mode, 'w')  # TODO or 'wb'?
         print >> writer, 'fee',
@@ -233,7 +266,8 @@
         from zope.app.mail.maildir import MaildirMessageWriter
         filename1 = '/path/to/maildir/tmp/1234500002.4242.myhostname'
         filename2 = '/path/to/maildir/new/1234500002.4242.myhostname'
-        writer = MaildirMessageWriter(filename1, filename2)
+        fd = FakeFile(filename1, 'w')
+        writer = MaildirMessageWriter(fd, filename1, filename2)
         writer.commit()
         self.assertEquals(writer._fd._closed, True)
         self.assert_((filename1, filename2)



More information about the Zope3-Checkins mailing list