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

[Backport of rev52776 from the trunk.]

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+')
+                if hasattr(os, 'link'):
-                try:
+                    os.link(os.path.join(self._path, str(key)),
-                    if self._locked:
+                            os.path.join(self._path, str(prev + 1)))
-                        _lock_file(f)
+                    os.unlink(os.path.join(self._path, str(key)))
-                    try:
+                else:
-                        if hasattr(os, 'link'):
+                    os.rename(os.path.join(self._path, str(key)),
-                            os.link(os.path.join(self._path, str(key)),
+                              os.path.join(self._path, str(prev + 1)))
-                                    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()
             prev += 1
         self._next_key = prev + 1
         if len(changes) == 0:

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')
 

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