Switch mbox locking from flock(2) to posix lockf(2)

flock() locks aren't portable; lockf() locks are.

5 files changed, 24 insertions, 35 deletions

diff --git a/TODO b/TODO
index fe8ee64..02af4e8 100644
--- a/TODO
+++ b/TODO
@@ -6,12 +6,6 @@ Gracefully close IMAP connection upon unexptected error (currently archivemail
 just terminates). 
 
 LOCKING & Co:
-* switch to or add fcntl() locking; when combining with flock, be careful not to
-  break on Solaris/FreeBSD, where flock() is just fcntl() so using both would
-  block. 
-* Ensure that we don't accidently lose fcntl locks when closing some file
-  descriptor; this applies even for flock, since again, it might be emulated
-  with fcntl
 * Block signals while writing changed mailbox back. Also, we probably shouldn't
   use shutil.copy2() for this; at least we have to handle symlinked targets in a
   sane way, see Debian bug #349068.  mbox_sync_mailbox() in the mutt code might
diff --git a/archivemail.1 b/archivemail.1
index 90c3393..5baa89d 100644
--- a/archivemail.1
+++ b/archivemail.1
@@ -3,7 +3,7 @@
 .\" <http://shell.ipoline.com/~elmert/comp/docbook2X/> 
 .\" Please send any bug reports, improvements, comments, patches, 
 .\" etc. to Steve Cheng <steve@ggi-project.org>.
-.TH "ARCHIVEMAIL" "1" "08 April 2008" "SP" ""
+.TH "ARCHIVEMAIL" "1" "03 March 2009" "SP" ""
 
 .SH NAME
 archivemail \- archive and compress your old email
@@ -258,7 +258,7 @@ Display brief summary information about how to run \fBarchivemail\fR\&.
 When reading an \fBmbox\fR-format mailbox, \fBarchivemail\fR will
 create a lockfile with the extension \fI\&.lock\fR so that
 procmail will not deliver to the mailbox while it is being processed. It will
-also create an advisory lock on the mailbox using \fBflock\fR(2)\&.
+also create an advisory lock on the mailbox using \fBlockf\fR(2)\&.
 \fBarchivemail\fR will also complain and abort if a 3rd-party modifies the
 mailbox while it is being read.
 .PP
diff --git a/archivemail.py b/archivemail.py
index dd2f2b4..0e89351 100755
--- a/archivemail.py
+++ b/archivemail.py
@@ -351,15 +351,15 @@ class Mbox(mailbox.UnixMailbox):
         os.utime(self.mbox_file_name, (self.original_atime,  \
             self.original_mtime)) 
 
-    def exclusive_lock(self):
-        """Set an advisory lock on the 'mbox' mailbox"""
+    def posix_lock(self):
+        """Set an exclusive posix lock on the 'mbox' mailbox"""
         vprint("obtaining exclusive lock on file '%s'" % self.mbox_file_name)
-        fcntl.flock(self.mbox_file.fileno(), fcntl.LOCK_EX)
+        fcntl.lockf(self.mbox_file, fcntl.LOCK_EX|fcntl.LOCK_NB)
 
-    def exclusive_unlock(self):
-        """Unset any advisory lock on the 'mbox' mailbox"""
-        vprint("dropping exclusive lock on file '%s'" % self.mbox_file_name)
-        fcntl.flock(self.mbox_file.fileno(), fcntl.LOCK_UN)
+    def posix_unlock(self):
+        """Unset any posix lock on the 'mbox' mailbox"""
+        vprint("dropping posix lock on file '%s'" % self.mbox_file_name)
+        fcntl.lockf(self.mbox_file, fcntl.LOCK_UN)
 
     def dotlock_lock(self):
         """Create a dotlock file on the 'mbox' mailbox"""
@@ -1133,7 +1133,7 @@ def _archive_mbox(mailbox_name, final_archive_name):
         archive = ArchiveMbox(final_archive_name)
 
     original.dotlock_lock()
-    original.exclusive_lock()
+    original.posix_lock()
     msg = original.next()
     if not msg and (original.starting_size > 0):
         user_error("'%s' is not a valid mbox-format mailbox" % mailbox_name)
@@ -1164,7 +1164,7 @@ def _archive_mbox(mailbox_name, final_archive_name):
         archive.finalise()
     if retain:
         retain.finalise()
-    original.exclusive_unlock()
+    original.posix_unlock()
     original.close()
     original.reset_timestamps()
     original.dotlock_unlock()
diff --git a/archivemail.sgml b/archivemail.sgml
index 93db525..f93a25f 100644
--- a/archivemail.sgml
+++ b/archivemail.sgml
@@ -1,7 +1,7 @@
 <!DOCTYPE RefEntry PUBLIC "-//Davenport//DTD DocBook V3.0//EN" [
 
-<!ENTITY flock "<CiteRefEntry>
-<RefEntryTitle><Command/flock/</RefEntryTitle>
+<!ENTITY lockf "<CiteRefEntry>
+<RefEntryTitle><Command/lockf/</RefEntryTitle>
 <ManVolNum/2/</CiteRefEntry>">
 
 <!ENTITY gzip "<CiteRefEntry>
@@ -410,7 +410,7 @@ Display brief summary information about how to run <Command/archivemail/.
 When reading an <application/mbox/-format mailbox, <command/archivemail/ will
 create a lockfile with the extension <filename>.lock</filename> so that
 procmail will not deliver to the mailbox while it is being processed. It will
-also create an advisory lock on the mailbox using &flock;.
+also create an advisory lock on the mailbox using &lockf;.
 <command/archivemail/ will also complain and abort if a 3rd-party modifies the
 mailbox while it is being read.
 </Para>
diff --git a/test_archivemail.py b/test_archivemail.py
index b2b244b..a2d4767 100755
--- a/test_archivemail.py
+++ b/test_archivemail.py
@@ -120,20 +120,15 @@ class TestMboxDotlock(TestCaseInTempdir):
         self.mbox.dotlock_unlock()
         assert(not os.path.isfile(lock))
 
-class TestMboxExclusiveLock(TestCaseInTempdir):
+class TestMboxPosixLock(TestCaseInTempdir):
     def setUp(self):
-        super(TestMboxExclusiveLock, self).setUp()
+        super(TestMboxPosixLock, self).setUp()
         self.mbox_name = make_mbox()
         self.mbox = archivemail.Mbox(self.mbox_name)
 
-    def testExclusiveLock(self):
-        """exclusive_lock/unlock should create/delete an advisory lock"""
+    def testPosixLock(self):
+        """posix_lock/unlock should create/delete an advisory lock"""
         
-        # We're using flock(2) locks; these aren't completely portable, and on
-        # some systems (e.g. Solaris) they may be emulated with fcntl(2) locks,
-        # which have pretty different semantics.  We could test real flock
-        # locks within this process, but that doesn't work for fcntl locks.  
-        #
         # The following code snippet heavily lends from the Python 2.5 mailbox
         # unittest.
         # BEGIN robbery:
@@ -143,29 +138,29 @@ class TestMboxExclusiveLock(TestCaseInTempdir):
         pid = os.fork()
         if pid == 0:
             # In the child, lock the mailbox.
-            self.mbox.exclusive_lock()
+            self.mbox.posix_lock()
             time.sleep(2)
-            self.mbox.exclusive_unlock()
+            self.mbox.posix_unlock()
             os._exit(0)
 
         # In the parent, sleep a bit to give the child time to acquire
         # the lock.
         time.sleep(0.5)
-        # The parent's file self.mbox.mbox_file shares flock locks with the
+        # The parent's file self.mbox.mbox_file shares fcntl locks with the
         # duplicated FD in the child; reopen it so we get a different file
         # table entry.
         file = open(self.mbox_name, "r+")
         lock_nb = fcntl.LOCK_EX | fcntl.LOCK_NB
         fd = file.fileno()
         try:
-            self.assertRaises(IOError, fcntl.flock, fd, lock_nb)
+            self.assertRaises(IOError, fcntl.lockf, fd, lock_nb)
 
         finally:
             # Wait for child to exit.  Locking should now succeed.
             exited_pid, status = os.waitpid(pid, 0)
 
-        fcntl.flock(fd, lock_nb)
-        fcntl.flock(fd, fcntl.LOCK_UN)
+        fcntl.lockf(fd, lock_nb)
+        fcntl.lockf(fd, fcntl.LOCK_UN)
         # END robbery