From d706409c595a5994371d2b238b2a5e77be171f13 Mon Sep 17 00:00:00 2001 From: Nikolaus Schulz Date: Tue, 3 Mar 2009 04:33:09 +0100 Subject: Switch mbox locking from flock(2) to posix lockf(2) flock() locks aren't portable; lockf() locks are. --- TODO | 6 ------ archivemail.1 | 4 ++-- archivemail.py | 18 +++++++++--------- archivemail.sgml | 6 +++--- test_archivemail.py | 25 ++++++++++--------------- 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 @@ .\" .\" Please send any bug reports, improvements, comments, patches, .\" etc. to Steve Cheng . -.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 @@ - + + "> @@ -410,7 +410,7 @@ Display brief summary information about how to run .lock 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;. 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 -- cgit v1.2.3