From: Andrew M. Kuchling <amk@amk.ca>
Date: Fri, 17 Nov 2006 16:16:28 +0000 (+0000)
Subject: Remove locking of individual message files in MH.pack().
X-Git-Tag: v2.5.1c1~248
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=830358af099ebafe0e32fb4bcbcf931d608664b5;p=python

Remove locking of individual message files in MH.pack().

[Backport of rev52776 from the trunk.]
---

diff --git a/Lib/mailbox.py b/Lib/mailbox.py
index c6b0fa00e3..108d874fe0 100755
--- a/Lib/mailbox.py
+++ b/Lib/mailbox.py
@@ -1054,27 +1054,13 @@ class MH(Mailbox):
         for key in self.iterkeys():
             if key - 1 != prev:
                 changes.append((key, prev + 1))
-                f = open(os.path.join(self._path, str(key)), 'r+')
-                try:
-                    if self._locked:
-                        _lock_file(f)
-                    try:
-                        if hasattr(os, 'link'):
-                            os.link(os.path.join(self._path, str(key)),
-                                    os.path.join(self._path, str(prev + 1)))
-                            if sys.platform == 'os2emx':
-                                # cannot unlink an open file on OS/2
-                                f.close()
-                            os.unlink(os.path.join(self._path, str(key)))
-                        else:
-                            f.close()
-                            os.rename(os.path.join(self._path, str(key)),
-                                      os.path.join(self._path, str(prev + 1)))
-                    finally:
-                        if self._locked:
-                            _unlock_file(f)
-                finally:
-                    f.close()
+                if hasattr(os, 'link'):
+                    os.link(os.path.join(self._path, str(key)),
+                            os.path.join(self._path, str(prev + 1)))
+                    os.unlink(os.path.join(self._path, str(key)))
+                else:
+                    os.rename(os.path.join(self._path, str(key)),
+                              os.path.join(self._path, str(prev + 1)))
             prev += 1
         self._next_key = prev + 1
         if len(changes) == 0:
diff --git a/Lib/test/test_mailbox.py b/Lib/test/test_mailbox.py
index aaf4097122..264e237221 100644
--- a/Lib/test/test_mailbox.py
+++ b/Lib/test/test_mailbox.py
@@ -887,6 +887,21 @@ class TestMH(TestMailbox):
         self.assert_(self._box.get_sequences() ==
                      {'foo':[1, 2, 3], 'unseen':[1], 'bar':[3], 'replied':[3]})
 
+        # Test case for packing while holding the mailbox locked.
+        key0 = self._box.add(msg1)
+        key1 = self._box.add(msg1)
+        key2 = self._box.add(msg1)
+        key3 = self._box.add(msg1)
+
+        self._box.remove(key0)
+        self._box.remove(key2)
+        self._box.lock()
+        self._box.pack()
+        self._box.unlock()
+        self.assert_(self._box.get_sequences() ==
+                     {'foo':[1, 2, 3, 4, 5],
+                      'unseen':[1], 'bar':[3], 'replied':[3]})
+        
     def _get_lock_path(self):
         return os.path.join(self._path, '.mh_sequences.lock')
 
diff --git a/Misc/NEWS b/Misc/NEWS
index 221acbb033..8324a055ac 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -158,6 +158,10 @@ Library
   been physically written to disk after calling .flush() or
   .close(). (Patch by David Watson.)
 
+- mailbox.py: Change MH.pack() to not lock individual message files; this
+  wasn't consistent with existing implementations of message packing, and
+  was buggy on some platforms.
+  
 - Bug #1576241: fix functools.wraps() to work on built-in functions.
 
 - Patch #1574068: fix urllib/urllib2 to not insert line breaks when