From 1ad73a52bb9d82ef3af1e34ad9ef66ac2eda2909 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Wed, 10 Oct 2007 23:22:03 -0400 Subject: General cleanups some of which is even tested . Mailman.LockFile module is moved to Mailman.lockfile. Remove a few more MailList methods that aren't used any more, e.g. the lock related stuff, the Save() and CheckValues() methods, as well as ChangeMemberName(). Add a missing import to lifecycle.py. We no longer need withlist to unlock the mailing list. Also, expose config.db.flush() in the namespace of withlist directly, under 'flush'. --- Mailman/Archiver/HyperArch.py | 6 +- Mailman/Archiver/HyperDatabase.py | 4 +- Mailman/Handlers/Scrubber.py | 11 +- Mailman/LockFile.py | 575 ------------------------------------- Mailman/MTA/Manual.py | 2 +- Mailman/MTA/Postfix.py | 16 +- Mailman/MailList.py | 104 ------- Mailman/app/lifecycle.py | 1 + Mailman/bin/arch.py | 87 +++--- Mailman/bin/gate_news.py | 21 +- Mailman/bin/mailmanctl.py | 8 +- Mailman/bin/withlist.py | 44 +-- Mailman/database/__init__.py | 9 +- Mailman/database/usermanager.py | 1 - Mailman/lockfile.py | 583 ++++++++++++++++++++++++++++++++++++++ Mailman/queue/archive.py | 16 +- Mailman/queue/bounce.py | 1 - Mailman/queue/command.py | 36 +-- Mailman/queue/incoming.py | 24 +- Mailman/queue/outgoing.py | 1 - Mailman/tests/test_lockfile.py | 2 +- 21 files changed, 687 insertions(+), 865 deletions(-) delete mode 100644 Mailman/LockFile.py create mode 100644 Mailman/lockfile.py diff --git a/Mailman/Archiver/HyperArch.py b/Mailman/Archiver/HyperArch.py index 80d4ea7a4..7ce0905ca 100644 --- a/Mailman/Archiver/HyperArch.py +++ b/Mailman/Archiver/HyperArch.py @@ -43,10 +43,10 @@ from email.Errors import HeaderParseError from email.Header import decode_header, make_header from Mailman import Errors -from Mailman import LockFile from Mailman import MailList from Mailman import Utils from Mailman import i18n +from Mailman import lockfile from Mailman.Archiver import HyperDatabase from Mailman.Archiver import pipermail from Mailman.Mailbox import ArchiverMailbox @@ -786,12 +786,12 @@ class HyperArchive(pipermail.T): def GetArchLock(self): if self._lock_file: return 1 - self._lock_file = LockFile.LockFile( + self._lock_file = lockfile.LockFile( os.path.join(config.LOCK_DIR, self.maillist.fqdn_listname + '-arch.lock')) try: self._lock_file.lock(timeout=0.5) - except LockFile.TimeOutError: + except lockfile.TimeOutError: return 0 return 1 diff --git a/Mailman/Archiver/HyperDatabase.py b/Mailman/Archiver/HyperDatabase.py index 36c299846..3644fbc58 100644 --- a/Mailman/Archiver/HyperDatabase.py +++ b/Mailman/Archiver/HyperDatabase.py @@ -27,7 +27,7 @@ import errno # package/project modules # import pipermail -from Mailman import LockFile +from Mailman.lockfile import LockFile CACHESIZE = pipermail.CACHESIZE @@ -58,7 +58,7 @@ class DumbBTree: def __init__(self, path): self.current_index = 0 self.path = path - self.lockfile = LockFile.LockFile(self.path + ".lock") + self.lockfile = LockFile(self.path + ".lock") self.lock() self.__dirty = 0 self.dict = {} diff --git a/Mailman/Handlers/Scrubber.py b/Mailman/Handlers/Scrubber.py index 655742899..3f29fc02b 100644 --- a/Mailman/Handlers/Scrubber.py +++ b/Mailman/Handlers/Scrubber.py @@ -17,6 +17,8 @@ """Cleanse a message for archiving.""" +from __future__ import with_statement + import os import re import sha @@ -34,13 +36,13 @@ from email.generator import Generator from email.parser import HeaderParser from email.utils import make_msgid, parsedate -from Mailman import LockFile from Mailman import Message from Mailman import Utils from Mailman.Errors import DiscardMessage from Mailman.app.archiving import get_base_archive_url from Mailman.configuration import config from Mailman.i18n import _ +from Mailman.lockfile import LockFile # Path characters for common platforms pre = re.compile(r'[/\\:]') @@ -422,10 +424,7 @@ def save_attachment(mlist, msg, dir, filter_html=True): ext = '.bin' path = None # We need a lock to calculate the next attachment number - lockfile = os.path.join(fsdir, 'attachments.lock') - lock = LockFile.LockFile(lockfile) - lock.lock() - try: + with LockFile(os.path.join(fsdir, 'attachments.lock')): # Now base the filename on what's in the attachment, uniquifying it if # necessary. if not filename or config.SCRUBBER_DONT_USE_ATTACHMENT_FILENAME: @@ -461,8 +460,6 @@ def save_attachment(mlist, msg, dir, filter_html=True): extra = '-%04d' % counter else: break - finally: - lock.unlock() # `path' now contains the unique filename for the attachment. There's # just one more step we need to do. If the part is text/html and # ARCHIVE_HTML_SANITIZER is a string (which it must be or we wouldn't be diff --git a/Mailman/LockFile.py b/Mailman/LockFile.py deleted file mode 100644 index 9323b4895..000000000 --- a/Mailman/LockFile.py +++ /dev/null @@ -1,575 +0,0 @@ -# Copyright (C) 1998-2007 by the Free Software Foundation, Inc. -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, -# USA. - -"""Portable, NFS-safe file locking with timeouts. - -This code implements an NFS-safe file-based locking algorithm influenced by -the GNU/Linux open(2) manpage, under the description of the O_EXCL option. -From RH6.1: - - [...] O_EXCL is broken on NFS file systems, programs which rely on it - for performing locking tasks will contain a race condition. The - solution for performing atomic file locking using a lockfile is to - create a unique file on the same fs (e.g., incorporating hostname and - pid), use link(2) to make a link to the lockfile. If link() returns - 0, the lock is successful. Otherwise, use stat(2) on the unique file - to check if its link count has increased to 2, in which case the lock - is also successful. - -The assumption made here is that there will be no `outside interference', -e.g. no agent external to this code will have access to link() to the affected -lock files. - -LockFile objects support lock-breaking so that you can't wedge a process -forever. This is especially helpful in a web environment, but may not be -appropriate for all applications. - -Locks have a `lifetime', which is the maximum length of time the process -expects to retain the lock. It is important to pick a good number here -because other processes will not break an existing lock until the expected -lifetime has expired. Too long and other processes will hang; too short and -you'll end up trampling on existing process locks -- and possibly corrupting -data. In a distributed (NFS) environment, you also need to make sure that -your clocks are properly synchronized. -""" - -# This code has undergone several revisions, with contributions from Barry -# Warsaw, Thomas Wouters, Harald Meland, and John Viega. It should also work -# well outside of Mailman so it could be used for other Python projects -# requiring file locking. See the __main__ section at the bottom of the file -# for unit testing. - -import os -import time -import errno -import random -import socket -import logging -import datetime -import traceback - -# Units are floating-point seconds. -DEFAULT_LOCK_LIFETIME = datetime.timedelta(seconds=15) -# Allowable a bit of clock skew, in seconds. -CLOCK_SLOP = 10 -# This is appropriate for Mailman, but you may want to change this if you're -# using this code outside Mailman. -log = logging.getLogger('mailman.locks') - - - -# Exceptions that can be raised by this module -class LockError(Exception): - """Base class for all exceptions in this module.""" - -class AlreadyLockedError(LockError): - """An attempt is made to lock an already locked object.""" - -class NotLockedError(LockError): - """An attempt is made to unlock an object that isn't locked.""" - -class TimeOutError(LockError): - """The timeout interval elapsed before the lock succeeded.""" - - - -class LockFile: - """A portable way to lock resources by way of the file system. - - This class supports the following methods: - - __init__(lockfile[, lifetime]): - Create the resource lock using lockfile as the global lock file. Each - process laying claim to this resource lock will create their own - temporary lock files based on the path specified by lockfile. - Optional lifetime is a timedelta specifying the number of seconds the - process expects to hold the lock. - - set_lifetime(lifetime): - Set a new lock lifetime. This takes affect the next time the file is - locked, but does not refresh a locked file. - - get_lifetime(): - Return the lock's lifetime. - - refresh([newlifetime[, unconditionally]]): - Refreshes the lifetime of a locked file. Use this if you realize that - you need to keep a resource locked longer than you thought. With - optional newlifetime, set the lock's lifetime. Raises NotLockedError - if the lock is not set, unless optional unconditionally flag is set to - true. - - lock([timeout]): - Acquire the lock. This blocks until the lock is acquired unless - optional timeout is greater than 0, in which case, a TimeOutError is - raised when timeout number of seconds (or possibly more) expires - without lock acquisition. Raises AlreadyLockedError if the lock is - already set. - - unlock([unconditionally]): - Relinquishes the lock. Raises a NotLockedError if the lock is not - set, unless optional unconditionally is true. - - locked(): - Return true if the lock is set, otherwise false. To avoid race - conditions, this refreshes the lock (on set locks). - - """ - # XXX We need to watch out for two lock objects in the same process - # pointing to the same lock file. Without this, if you lock lf1 and do - # not lock lf2, lf2.locked() will still return true. NOTE: this gimmick - # probably does /not/ work in a multithreaded world, but we don't have to - # worry about that, do we? <1 wink>. - COUNTER = 0 - - def __init__(self, lockfile, lifetime=DEFAULT_LOCK_LIFETIME): - """Create the resource lock using lockfile as the global lock file. - - Each process laying claim to this resource lock will create their own - temporary lock files based on the path specified by lockfile. - Optional lifetime is the number of seconds the process expects to hold - the lock. Optional withlogging, when true, turns on lockfile logging - (see the module docstring for details). - """ - self._lockfile = lockfile - self._lifetime = lifetime - # This works because we know we're single threaded - self._counter = LockFile.COUNTER - LockFile.COUNTER += 1 - self._tmpfname = '%s.%s.%d.%d' % ( - lockfile, socket.gethostname(), os.getpid(), self._counter) - # For transferring ownership across a fork. - self._owned = True - - def __repr__(self): - return '' % ( - id(self), self._lockfile, - self.locked() and 'locked' or 'unlocked', - self._lifetime, os.getpid()) - - def set_lifetime(self, lifetime): - """Set a new lock lifetime. - - This takes affect the next time the file is locked, but does not - refresh a locked file. - """ - self._lifetime = lifetime - - def get_lifetime(self): - """Return the lock's lifetime.""" - return self._lifetime - - def refresh(self, newlifetime=None, unconditionally=False): - """Refreshes the lifetime of a locked file. - - Use this if you realize that you need to keep a resource locked longer - than you thought. With optional newlifetime, set the lock's lifetime. - Raises NotLockedError if the lock is not set, unless optional - unconditionally flag is set to true. - """ - if newlifetime is not None: - self.set_lifetime(newlifetime) - # Do we have the lock? As a side effect, this refreshes the lock! - if not self.locked() and not unconditionally: - raise NotLockedError('%s: %s' % (repr(self), self._read())) - - def lock(self, timeout=0): - """Acquire the lock. - - This blocks until the lock is acquired unless optional timeout is - greater than 0, in which case, a TimeOutError is raised when timeout - number of seconds (or possibly more) expires without lock acquisition. - Raises AlreadyLockedError if the lock is already set. - """ - if timeout: - timeout_time = time.time() + timeout - # Make sure my temp lockfile exists, and that its contents are - # up-to-date (e.g. the temp file name, and the lock lifetime). - self._write() - # XXX This next call can fail with an EPERM. I have no idea why, but - # I'm nervous about wrapping this in a try/except. It seems to be a - # very rare occurence, only happens from cron, and (only?) on Solaris - # 2.6. - self._touch() - log.debug('laying claim: %s', self._lockfile) - # for quieting the logging output - loopcount = -1 - while True: - loopcount += 1 - # Create the hard link and test for exactly 2 links to the file - try: - os.link(self._tmpfname, self._lockfile) - # If we got here, we know we know we got the lock, and never - # had it before, so we're done. Just touch it again for the - # fun of it. - log.debug('got the lock: %s', self._lockfile) - self._touch() - break - except OSError, e: - # The link failed for some reason, possibly because someone - # else already has the lock (i.e. we got an EEXIST), or for - # some other bizarre reason. - if e.errno == errno.ENOENT: - # XXX in some Linux environments, it is possible to get - # an ENOENT, which is truly strange, because this means - # that self._tmpfname doesn't exist at the time of the - # os.link(), but self._write() is supposed to guarantee - # that this happens! I don't honestly know why this - # happens, but for now we just say we didn't acquire the - # lock, and try again next time. - pass - elif e.errno <> errno.EEXIST: - # Something very bizarre happened. Clean up our state and - # pass the error on up. - log.exception('unexpected link') - os.unlink(self._tmpfname) - raise - elif self._linkcount() <> 2: - # Somebody's messin' with us! Log this, and try again - # later. XXX should we raise an exception? - log.error('unexpected linkcount: %d', self._linkcount()) - elif self._read() == self._tmpfname: - # It was us that already had the link. - log.debug('already locked: %s', self._lockfile) - raise AlreadyLockedError - # otherwise, someone else has the lock - pass - # We did not acquire the lock, because someone else already has - # it. Have we timed out in our quest for the lock? - if timeout and timeout_time < time.time(): - os.unlink(self._tmpfname) - log.error('timed out') - raise TimeOutError - # Okay, we haven't timed out, but we didn't get the lock. Let's - # find if the lock lifetime has expired. - if time.time() > self._releasetime() + CLOCK_SLOP: - # Yes, so break the lock. - self._break() - log.error('lifetime has expired, breaking') - # Okay, someone else has the lock, our claim hasn't timed out yet, - # and the expected lock lifetime hasn't expired yet. So let's - # wait a while for the owner of the lock to give it up. - elif not loopcount % 100: - log.debug('waiting for claim: %s', self._lockfile) - self._sleep() - - def unlock(self, unconditionally=False): - """Unlock the lock. - - If we don't already own the lock (either because of unbalanced unlock - calls, or because the lock was stolen out from under us), raise a - NotLockedError, unless optional `unconditionally' is true. - """ - islocked = self.locked() - if not islocked and not unconditionally: - raise NotLockedError - # If we owned the lock, remove the global file, relinquishing it. - if islocked: - try: - os.unlink(self._lockfile) - except OSError, e: - if e.errno <> errno.ENOENT: - raise - # Remove our tempfile - try: - os.unlink(self._tmpfname) - except OSError, e: - if e.errno <> errno.ENOENT: - raise - log.debug('unlocked: %s', self._lockfile) - - def locked(self): - """Return true if we own the lock, false if we do not. - - Checking the status of the lock resets the lock's lifetime, which - helps avoid race conditions during the lock status test. - """ - # Discourage breaking the lock for a while. - try: - self._touch() - except OSError, e: - if e.errno == errno.EPERM: - # We can't touch the file because we're not the owner. I - # don't see how we can own the lock if we're not the owner. - return False - else: - raise - # XXX Can the link count ever be > 2? - if self._linkcount() <> 2: - return False - return self._read() == self._tmpfname - - def finalize(self): - log.debug('finalize: %s', self._lockfile) - self.unlock(unconditionally=True) - - def __del__(self): - log.debug('__del__: %s', self._lockfile) - if self._owned: - self.finalize() - - # Python 2.5 context manager protocol support. - def __enter__(self): - self.lock() - return self - - def __exit__(self, exc_type, exc_val, exc_tb): - self.unlock() - # Don't suppress any exception that might have occurred. - return False - - # Use these only if you're transfering ownership to a child process across - # a fork. Use at your own risk, but it should be race-condition safe. - # _transfer_to() is called in the parent, passing in the pid of the child. - # _take_possession() is called in the child, and blocks until the parent - # has transferred possession to the child. _disown() is used to set the - # _owned flag to false, and it is a disgusting wart necessary to make - # forced lock acquisition work in mailmanctl. :( - def _transfer_to(self, pid): - # First touch it so it won't get broken while we're fiddling about. - self._touch() - # Find out current claim's temp filename - winner = self._read() - # Now twiddle ours to the given pid - self._tmpfname = '%s.%s.%d' % ( - self._lockfile, socket.gethostname(), pid) - # Create a hard link from the global lock file to the temp file. This - # actually does things in reverse order of normal operation because we - # know that lockfile exists, and tmpfname better not! - os.link(self._lockfile, self._tmpfname) - # Now update the lock file to contain a reference to the new owner - self._write() - # Toggle off our ownership of the file so we don't try to finalize it - # in our __del__() - self._owned = False - # Unlink the old winner, completing the transfer - os.unlink(winner) - # And do some sanity checks - assert self._linkcount() == 2 - assert self.locked() - log.debug('transferred the lock: %s', self._lockfile) - - def _take_possession(self): - self._tmpfname = tmpfname = '%s.%s.%d' % ( - self._lockfile, socket.gethostname(), os.getpid()) - # Wait until the linkcount is 2, indicating the parent has completed - # the transfer. - while self._linkcount() <> 2 or self._read() <> tmpfname: - time.sleep(0.25) - log.debug('took possession of the lock: %s', self._lockfile) - - def _disown(self): - self._owned = False - - # - # Private interface - # - - def _write(self): - # Make sure it's group writable - fp = open(self._tmpfname, 'w') - try: - fp.write(self._tmpfname) - finally: - fp.close() - - def _read(self): - try: - fp = open(self._lockfile) - try: - filename = fp.read() - finally: - fp.close() - return filename - except EnvironmentError, e: - if e.errno <> errno.ENOENT: - raise - return None - - def _touch(self, filename=None): - expiration_date = datetime.datetime.now() + self._lifetime - t = time.mktime(expiration_date.timetuple()) - try: - # XXX We probably don't need to modify atime, but this is easier. - os.utime(filename or self._tmpfname, (t, t)) - except OSError, e: - if e.errno <> errno.ENOENT: - raise - - def _releasetime(self): - try: - return os.stat(self._lockfile).st_mtime - except OSError, e: - if e.errno <> errno.ENOENT: - raise - return -1 - - def _linkcount(self): - try: - return os.stat(self._lockfile).st_nlink - except OSError, e: - if e.errno <> errno.ENOENT: - raise - return -1 - - def _break(self): - # First, touch the global lock file. This reduces but does not - # eliminate the chance for a race condition during breaking. Two - # processes could both pass the test for lock expiry in lock() before - # one of them gets to touch the global lockfile. This shouldn't be - # too bad because all they'll do in this function is wax the lock - # files, not claim the lock, and we can be defensive for ENOENTs - # here. - # - # Touching the lock could fail if the process breaking the lock and - # the process that claimed the lock have different owners. We could - # solve this by set-uid'ing the CGI and mail wrappers, but I don't - # think it's that big a problem. - try: - self._touch(self._lockfile) - except OSError, e: - if e.errno <> errno.EPERM: - raise - # Get the name of the old winner's temp file. - winner = self._read() - # Remove the global lockfile, which actually breaks the lock. - try: - os.unlink(self._lockfile) - except OSError, e: - if e.errno <> errno.ENOENT: - raise - # Try to remove the old winner's temp file, since we're assuming the - # winner process has hung or died. Don't worry too much if we can't - # unlink their temp file -- this doesn't wreck the locking algorithm, - # but will leave temp file turds laying around, a minor inconvenience. - try: - if winner: - os.unlink(winner) - except OSError, e: - if e.errno <> errno.ENOENT: - raise - - def _sleep(self): - interval = random.random() * 2.0 + 0.01 - time.sleep(interval) - - - -# Unit test framework -def _dochild(): - prefix = '[%d]' % os.getpid() - # Create somewhere between 1 and 1000 locks - lockfile = LockFile('/tmp/LockTest', lifetime=120) - # Use a lock lifetime of between 1 and 15 seconds. Under normal - # situations, Mailman's usage patterns (untested) shouldn't be much longer - # than this. - workinterval = 5 * random.random() - hitwait = 20 * random.random() - print prefix, 'workinterval:', workinterval - islocked = False - t0 = 0 - t1 = 0 - t2 = 0 - try: - try: - t0 = time.time() - print prefix, 'acquiring...' - lockfile.lock() - print prefix, 'acquired...' - islocked = True - except TimeOutError: - print prefix, 'timed out' - else: - t1 = time.time() - print prefix, 'acquisition time:', t1-t0, 'seconds' - time.sleep(workinterval) - finally: - if islocked: - try: - lockfile.unlock() - t2 = time.time() - print prefix, 'lock hold time:', t2-t1, 'seconds' - except NotLockedError: - print prefix, 'lock was broken' - # wait for next web hit - print prefix, 'webhit sleep:', hitwait - time.sleep(hitwait) - - -def _seed(): - try: - fp = open('/dev/random') - d = fp.read(40) - fp.close() - except EnvironmentError, e: - if e.errno <> errno.ENOENT: - raise - import sha - d = sha.new(`os.getpid()`+`time.time()`).hexdigest() - random.seed(d) - - -def _onetest(): - loopcount = random.randint(1, 100) - for i in range(loopcount): - print 'Loop %d of %d' % (i+1, loopcount) - pid = os.fork() - if pid: - # parent, wait for child to exit - pid, status = os.waitpid(pid, 0) - else: - # child - _seed() - try: - _dochild() - except KeyboardInterrupt: - pass - os._exit(0) - - -def _reap(kids): - if not kids: - return - pid, status = os.waitpid(-1, os.WNOHANG) - if pid <> 0: - del kids[pid] - - -def _test(numtests): - kids = {} - for i in range(numtests): - pid = os.fork() - if pid: - # parent - kids[pid] = pid - else: - # child - _seed() - try: - _onetest() - except KeyboardInterrupt: - pass - os._exit(0) - # slightly randomize each kid's seed - while kids: - _reap(kids) - - -if __name__ == '__main__': - import sys - import random - _test(int(sys.argv[1])) diff --git a/Mailman/MTA/Manual.py b/Mailman/MTA/Manual.py index 953d46695..0d9a4bca7 100644 --- a/Mailman/MTA/Manual.py +++ b/Mailman/MTA/Manual.py @@ -98,7 +98,7 @@ equivalent) file by adding the following lines, and possibly running the def remove(mlist, cgi=False): - listname = mlist.internal_name() + listname = mlist.fqdn_listname fieldsz = len(listname) + len('-unsubscribe') if cgi: # If a list is being removed via the CGI, the best we can do is send diff --git a/Mailman/MTA/Postfix.py b/Mailman/MTA/Postfix.py index 6b43011ff..1712bb638 100644 --- a/Mailman/MTA/Postfix.py +++ b/Mailman/MTA/Postfix.py @@ -17,6 +17,8 @@ """Creation/deletion hooks for the Postfix MTA.""" +from __future__ import with_statement + import os import grp import pwd @@ -26,11 +28,11 @@ import logging from stat import * -from Mailman import LockFile from Mailman import Utils from Mailman.MTA.Utils import makealiases from Mailman.configuration import config from Mailman.i18n import _ +from Mailman.lockfile import LockFile LOCKFILE = os.path.join(config.LOCK_DIR, 'creator') ALIASFILE = os.path.join(config.DATA_DIR, 'aliases') @@ -66,10 +68,6 @@ def _update_maps(): -def makelock(): - return LockFile.LockFile(LOCKFILE) - - def _zapfile(filename): # Truncate the file w/o messing with the file permissions, but only if it # already exists. @@ -274,6 +272,7 @@ def create(mlist, cgi=False, nolock=False, quiet=False): # Acquire the global list database lock. quiet flag is ignored. lock = None if not nolock: + # XXX FIXME lock = makelock() lock.lock() # Do the aliases file, which always needs to be done @@ -339,9 +338,7 @@ def _do_remove(mlist, textfile): def remove(mlist, cgi=False): # Acquire the global list database lock - lock = makelock() - lock.lock() - try: + with LockFile(LOCKFILE): if config.USE_LMTP: _do_remove(mlist, TRPTFILE) else: @@ -350,8 +347,7 @@ def remove(mlist, cgi=False): _do_remove(mlist, VIRTFILE) # Regenerate the alias and map files _update_maps() - finally: - lock.unlock(unconditionally=True) + config.db.commit() diff --git a/Mailman/MailList.py b/Mailman/MailList.py index d90de18f0..335d82581 100644 --- a/Mailman/MailList.py +++ b/Mailman/MailList.py @@ -47,7 +47,6 @@ from email.Header import Header from email.Utils import getaddresses, formataddr, parseaddr from Mailman import Errors -from Mailman import LockFile from Mailman import Utils from Mailman import Version from Mailman import database @@ -131,36 +130,6 @@ class MailList(object, Archiver, Digester, SecurityManager, Bouncer): def __repr__(self): return '' % (self.fqdn_listname, id(self)) - - # - # Lock management - # - def _make_lock(self, name, lock=False): - self._lock = LockFile.LockFile( - os.path.join(config.LOCK_DIR, name) + '.lock', - lifetime=config.LIST_LOCK_LIFETIME) - if lock: - self._lock.lock() - - def Lock(self, timeout=0): - self._lock.lock(timeout) - self._memberadaptor.lock() - # Must reload our database for consistency. Watch out for lists that - # don't exist. - try: - self.Load() - except Exception: - self.Unlock() - raise - - def Unlock(self): - self._lock.unlock(unconditionally=True) - self._memberadaptor.unlock() - - def Locked(self): - return self._lock.locked() - - def GetConfirmJoinSubject(self, listname, cookie): if config.VERP_CONFIRMATIONS and cookie: @@ -257,59 +226,6 @@ class MailList(object, Archiver, Digester, SecurityManager, Bouncer): if value: return value - - def Save(self): - # Refresh the lock, just to let other processes know we're still - # interested in it. This will raise a NotLockedError if we don't have - # the lock (which is a serious problem!). TBD: do we need to be more - # defensive? - self._lock.refresh() - # The member adaptor may have its own save operation - self._memberadaptor.save() - self.CheckHTMLArchiveDir() - - def Load(self): - self._memberadaptor.load() - - - - # - # Sanity checks - # - def CheckValues(self): - """Normalize selected values to known formats.""" - if '' in urlparse(self.web_page_url)[:2]: - # Either the "scheme" or the "network location" part of the parsed - # URL is empty; substitute faulty value with (hopefully sane) - # default. Note that DEFAULT_URL is obsolete. - self.web_page_url = ( - config.DEFAULT_URL or - config.DEFAULT_URL_PATTERN % config.DEFAULT_URL_HOST) - if self.web_page_url and self.web_page_url[-1] <> '/': - self.web_page_url = self.web_page_url + '/' - # Legacy reply_to_address could be an illegal value. We now verify - # upon setting and don't check it at the point of use. - try: - if self.reply_to_address.strip() and self.reply_goes_to_list: - Utils.ValidateEmail(self.reply_to_address) - except Errors.EmailAddressError: - elog.error('Bad reply_to_address "%s" cleared for list: %s', - self.reply_to_address, self.internal_name()) - self.reply_to_address = '' - self.reply_goes_to_list = 0 - # Legacy topics may have bad regular expressions in their patterns - goodtopics = [] - for name, pattern, desc, emptyflag in self.topics: - try: - orpattern = OR.join(pattern.splitlines()) - re.compile(orpattern) - except (re.error, TypeError): - elog.error('Bad topic pattern "%s" for list: %s', - orpattern, self.internal_name()) - else: - goodtopics.append((name, pattern, desc, emptyflag)) - self.topics = goodtopics - # # Membership management front-ends and assertion checks @@ -479,26 +395,6 @@ class MailList(object, Archiver, Digester, SecurityManager, Bouncer): raise Errors.MMNeedApproval, _( 'unsubscriptions require moderator approval') - def ChangeMemberName(self, addr, name, globally): - self.setMemberName(addr, name) - if not globally: - return - for listname in config.list_manager.names: - # Don't bother with ourselves - if listname == self.internal_name(): - continue - mlist = MailList(listname, lock=0) - if mlist.host_name <> self.host_name: - continue - if not mlist.isMember(addr): - continue - mlist.Lock() - try: - mlist.setMemberName(addr, name) - mlist.Save() - finally: - mlist.Unlock() - def ChangeMemberAddress(self, oldaddr, newaddr, globally): # Changing a member address consists of verifying the new address, # making sure the new address isn't already a member, and optionally diff --git a/Mailman/app/lifecycle.py b/Mailman/app/lifecycle.py index 8abb14c69..251ded2b8 100644 --- a/Mailman/app/lifecycle.py +++ b/Mailman/app/lifecycle.py @@ -18,6 +18,7 @@ """Application level list creation.""" import os +import sys import shutil import logging diff --git a/Mailman/bin/arch.py b/Mailman/bin/arch.py index a7929e407..09ca4d914 100644 --- a/Mailman/bin/arch.py +++ b/Mailman/bin/arch.py @@ -15,6 +15,8 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, # USA. +from __future__ import with_statement + import os import sys import errno @@ -25,9 +27,10 @@ from Mailman import Errors from Mailman import Version from Mailman import i18n from Mailman.Archiver.HyperArch import HyperArchive -from Mailman.LockFile import LockFile +from Mailman.Defaults import hours from Mailman.MailList import MailList from Mailman.configuration import config +from Mailman.lockfile import LockFile _ = i18n._ __i18n_templates__ = True @@ -119,53 +122,41 @@ def main(): # really don't know how long it will take. # # XXX processUnixMailbox() should refresh the lock. - # - # XXX This may not be necessary because I think we lay claim to the - # list lock up above, although that may be too short to be of use (and - # maybe we don't really want to lock the list anyway). - lockfile = os.path.join(config.LOCK_DIR, mlist._internal_name) + \ - '.archiver.lock' - # set the lock lifetime to 3 hours. XXX is this reasonable??? - lock = LockFile(lockfile, lifetime=3*60*60) - lock.lock() - # Maybe wipe the old archives - if opts.wipe: - if mlist.scrub_nondigest: - # TK: save the attachments dir because they are not in mbox - saved = False - atchdir = os.path.join(mlist.archive_dir(), 'attachments') - savedir = os.path.join(mlist.archive_dir() + '.mbox', - 'attachments') - try: - os.rename(atchdir, savedir) - saved = True - except OSError, e: - if e.errno <> errno.ENOENT: - raise - shutil.rmtree(mlist.archive_dir()) - if mlist.scrub_nondigest and saved: - os.renames(savedir, atchdir) - try: - fp = open(mbox) - except IOError, e: - if e.errno == errno.ENOENT: - print >> sys.stderr, _('Cannot open mbox file: $mbox') - else: - print >> sys.stderr, e - sys.exit(1) - - archiver = HyperArchive(mlist) - archiver.VERBOSE = opts.verbose - try: - archiver.processUnixMailbox(fp, opts.start, opts.end) - finally: - archiver.close() - fp.close() - finally: - if lock: - lock.unlock() - if mlist: - mlist.Unlock() + with LockFile(os.path.join(mlist.full_path, '.archiver.lck'), + lifetime=int(hours(3))): + # Maybe wipe the old archives + if opts.wipe: + if mlist.scrub_nondigest: + # TK: save the attachments dir because they are not in mbox + saved = False + atchdir = os.path.join(mlist.archive_dir(), 'attachments') + savedir = os.path.join(mlist.archive_dir() + '.mbox', + 'attachments') + try: + os.rename(atchdir, savedir) + saved = True + except OSError, e: + if e.errno <> errno.ENOENT: + raise + shutil.rmtree(mlist.archive_dir()) + if mlist.scrub_nondigest and saved: + os.renames(savedir, atchdir) + try: + fp = open(mbox) + except IOError, e: + if e.errno == errno.ENOENT: + print >> sys.stderr, _('Cannot open mbox file: $mbox') + else: + print >> sys.stderr, e + sys.exit(1) + + archiver = HyperArchive(mlist) + archiver.VERBOSE = opts.verbose + try: + archiver.processUnixMailbox(fp, opts.start, opts.end) + finally: + archiver.close() + fp.close() diff --git a/Mailman/bin/gate_news.py b/Mailman/bin/gate_news.py index da78b6068..6fc8139c6 100644 --- a/Mailman/bin/gate_news.py +++ b/Mailman/bin/gate_news.py @@ -15,6 +15,8 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, # USA. +from __future__ import with_statement + import os import sys import time @@ -26,11 +28,11 @@ import optparse import email.Errors from email.Parser import Parser -from Mailman import LockFile from Mailman import MailList from Mailman import Message from Mailman import Utils from Mailman import Version +from Mailman import lockfile from Mailman import loginit from Mailman.configuration import config from Mailman.i18n import _ @@ -210,7 +212,7 @@ def process_lists(glock): # loop over range, and this will not include the last # element in the list. poll_newsgroup(mlist, conn, start, last + 1, glock) - except LockFile.TimeOutError: + except lockfile.TimeOutError: log.error('Could not acquire list lock: %s', listname) finally: if mlist.Locked(): @@ -230,19 +232,14 @@ def main(): loginit.initialize(propagate=True) log = logging.getLogger('mailman.fromusenet') - lock = LockFile.LockFile(GATENEWS_LOCK_FILE, - # It's okay to hijack this - lifetime=LOCK_LIFETIME) try: - lock.lock(timeout=0.5) + with lockfile.LockFile(GATENEWS_LOCK_FILE, + # It's okay to hijack this + lifetime=LOCK_LIFETIME): + process_lists(lock) + clearcache() except LockFile.TimeOutError: log.error('Could not acquire gate_news lock') - return - try: - process_lists(lock) - finally: - clearcache() - lock.unlock(unconditionally=True) diff --git a/Mailman/bin/mailmanctl.py b/Mailman/bin/mailmanctl.py index f6190f685..07716029b 100644 --- a/Mailman/bin/mailmanctl.py +++ b/Mailman/bin/mailmanctl.py @@ -27,9 +27,9 @@ import optparse from Mailman import Defaults from Mailman import Errors -from Mailman import LockFile from Mailman import Utils from Mailman import Version +from Mailman import lockfile from Mailman import loginit from Mailman.configuration import config from Mailman.i18n import _ @@ -197,11 +197,11 @@ def qrunner_state(): def acquire_lock_1(force): # Be sure we can acquire the master qrunner lock. If not, it means some # other master qrunner daemon is already going. - lock = LockFile.LockFile(config.LOCK_FILE, LOCK_LIFETIME) + lock = lockfile.LockFile(config.LOCK_FILE, LOCK_LIFETIME) try: lock.lock(0.1) return lock - except LockFile.TimeOutError: + except lockfile.TimeOutError: if not force: raise # Force removal of lock first @@ -216,7 +216,7 @@ def acquire_lock(force): try: lock = acquire_lock_1(force) return lock - except LockFile.TimeOutError: + except lockfile.TimeOutError: status = qrunner_state() if status == 1: # host matches and proc exists diff --git a/Mailman/bin/withlist.py b/Mailman/bin/withlist.py index cf40ddabd..dc820eb90 100644 --- a/Mailman/bin/withlist.py +++ b/Mailman/bin/withlist.py @@ -17,7 +17,6 @@ import os import sys -import atexit import optparse from Mailman import Errors @@ -31,20 +30,6 @@ __i18n_templates__ = True LAST_MLIST = None VERBOSE = True -LOCK = False - - - -def exitfunc(mlist): - """Unlock a locked list, but do not implicitly Save() it.""" - if mlist.Locked(): - if VERBOSE: - listname = mlist.fqdn_listname - print >> sys.stderr, _( - 'Unlocking (but not saving) list: $listname') - mlist.Unlock() - if VERBOSE: - print >> sys.stderr, _('Finalizing') @@ -54,18 +39,12 @@ def do_list(listname, args, func): if '@' not in listname: listname += '@' + config.DEFAULT_EMAIL_HOST - if VERBOSE: - print >> sys.stderr, _('Loading list $listname'), - if LOCK: - print >> sys.stderr, _('(locked)') - else: - print >> sys.stderr, _('(unlocked)') - mlist = config.db.list_manager.get(listname) if mlist is None: print >> sys.stderr, _('Unknown list: $listname') else: - atexit.register(exitfunc, mlist) + if VERBOSE: + print >> sys.stderr, _('Loaded list: $listname') LAST_MLIST = mlist # Try to import the module and run the callable. if func: @@ -107,7 +86,7 @@ Now, from the command line you can print the list's posting address by running the following from the command line: % bin/withlist -r listaddr mylist - Loading list: mylist (unlocked) + Loading list: mylist Importing listaddr ... Running listaddr.listaddr() ... mylist@myhost.com @@ -115,7 +94,7 @@ the following from the command line: And you can print the list's request address by running: % bin/withlist -r listaddr.requestaddr mylist - Loading list: mylist (unlocked) + Loading list: mylist Importing listaddr ... Running listaddr.requestaddr() ... mylist-request@myhost.com @@ -136,15 +115,6 @@ called 'changepw.py': and run this from the command line: % bin/withlist -l -r changepw mylist somebody@somewhere.org foobar""")) - parser.add_option('-l', '--lock', - default=False, action='store_true', help=_("""\ -Lock the list when opening. Normally the list is opened unlocked (e.g. for -read-only operations). You can always lock the file after the fact by typing -'m.Lock()' - -Note that if you use this option, you should explicitly call m.Save() before -exiting, since the interpreter's clean up procedure will not automatically -save changes to the IMailingList object (but it will unlock the list).""")) parser.add_option('-i', '--interactive', default=None, action='store_true', help=_("""\ Leaves you at an interactive prompt after all other processing is complete. @@ -180,14 +150,12 @@ the results.""")) def main(): - global LAST_MLIST, LOCK, VERBOSE + global LAST_MLIST, VERBOSE parser, opts, args = parseargs() initialize(opts.config, not opts.quiet) VERBOSE = not opts.quiet - LOCK = opts.lock - # The default for interact is true unless -r was given if opts.interactive is None: if not opts.run: @@ -241,5 +209,5 @@ def main(): "The variable 'm' is the $listname mailing list") else: banner = interact.DEFAULT_BANNER - overrides = dict(m=LAST_MLIST, r=r) + overrides = dict(m=LAST_MLIST, r=r, flush=config.db.flush) interact.interact(upframe=False, banner=banner, overrides=overrides) diff --git a/Mailman/database/__init__.py b/Mailman/database/__init__.py index 9c3cf39ea..e9c338952 100644 --- a/Mailman/database/__init__.py +++ b/Mailman/database/__init__.py @@ -56,13 +56,12 @@ class StockDatabase: self.requests = None def initialize(self): - from Mailman.LockFile import LockFile from Mailman.configuration import config from Mailman.database import model - # Serialize this so we don't get multiple processes trying to create the - # database at the same time. - lockfile = os.path.join(config.LOCK_DIR, '') - with LockFile(lockfile): + from Mailman.lockfile import LockFile + # Serialize this so we don't get multiple processes trying to create + # the database at the same time. + with LockFile(os.path.join(config.LOCK_DIR, 'dbcreate.lck')): model.initialize() self.list_manager = ListManager() self.user_manager = UserManager() diff --git a/Mailman/database/usermanager.py b/Mailman/database/usermanager.py index 038427879..1958080fd 100644 --- a/Mailman/database/usermanager.py +++ b/Mailman/database/usermanager.py @@ -25,7 +25,6 @@ from elixir import * from zope.interface import implements from Mailman import Errors -from Mailman.LockFile import LockFile from Mailman.configuration import config from Mailman.database.model import * from Mailman.interfaces import IUserManager diff --git a/Mailman/lockfile.py b/Mailman/lockfile.py new file mode 100644 index 000000000..7db746952 --- /dev/null +++ b/Mailman/lockfile.py @@ -0,0 +1,583 @@ +# Copyright (C) 1998-2007 by the Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, +# USA. + +"""Portable, NFS-safe file locking with timeouts. + +This code implements an NFS-safe file-based locking algorithm influenced by +the GNU/Linux open(2) manpage, under the description of the O_EXCL option. +From RH6.1: + + [...] O_EXCL is broken on NFS file systems, programs which rely on it + for performing locking tasks will contain a race condition. The + solution for performing atomic file locking using a lockfile is to + create a unique file on the same fs (e.g., incorporating hostname and + pid), use link(2) to make a link to the lockfile. If link() returns + 0, the lock is successful. Otherwise, use stat(2) on the unique file + to check if its link count has increased to 2, in which case the lock + is also successful. + +The assumption made here is that there will be no `outside interference', +e.g. no agent external to this code will have access to link() to the affected +lock files. + +LockFile objects support lock-breaking so that you can't wedge a process +forever. This is especially helpful in a web environment, but may not be +appropriate for all applications. + +Locks have a `lifetime', which is the maximum length of time the process +expects to retain the lock. It is important to pick a good number here +because other processes will not break an existing lock until the expected +lifetime has expired. Too long and other processes will hang; too short and +you'll end up trampling on existing process locks -- and possibly corrupting +data. In a distributed (NFS) environment, you also need to make sure that +your clocks are properly synchronized. +""" + +__metaclass__ = type +__all__ = [ + 'LockError', + 'AlreadyLockedError', + 'NotLockedError', + 'LockFile', + ] + +# This code has undergone several revisions, with contributions from Barry +# Warsaw, Thomas Wouters, Harald Meland, and John Viega. It should also work +# well outside of Mailman so it could be used for other Python projects +# requiring file locking. See the __main__ section at the bottom of the file +# for unit testing. + +import os +import time +import errno +import random +import socket +import logging +import datetime +import traceback + +# Units are floating-point seconds. +DEFAULT_LOCK_LIFETIME = datetime.timedelta(seconds=15) +# Allowable a bit of clock skew, in seconds. +CLOCK_SLOP = 10 +# This is appropriate for Mailman, but you may want to change this if you're +# using this code outside Mailman. +log = logging.getLogger('mailman.locks') + + + +# Exceptions that can be raised by this module +class LockError(Exception): + """Base class for all exceptions in this module.""" + +class AlreadyLockedError(LockError): + """An attempt is made to lock an already locked object.""" + +class NotLockedError(LockError): + """An attempt is made to unlock an object that isn't locked.""" + +class TimeOutError(LockError): + """The timeout interval elapsed before the lock succeeded.""" + + + +class LockFile: + """A portable way to lock resources by way of the file system. + + This class supports the following methods: + + __init__(lockfile[, lifetime]): + Create the resource lock using lockfile as the global lock file. Each + process laying claim to this resource lock will create their own + temporary lock files based on the path specified by lockfile. + Optional lifetime is a timedelta specifying the number of seconds the + process expects to hold the lock. + + set_lifetime(lifetime): + Set a new lock lifetime. This takes affect the next time the file is + locked, but does not refresh a locked file. + + get_lifetime(): + Return the lock's lifetime. + + refresh([newlifetime[, unconditionally]]): + Refreshes the lifetime of a locked file. Use this if you realize that + you need to keep a resource locked longer than you thought. With + optional newlifetime, set the lock's lifetime. Raises NotLockedError + if the lock is not set, unless optional unconditionally flag is set to + true. + + lock([timeout]): + Acquire the lock. This blocks until the lock is acquired unless + optional timeout is greater than 0, in which case, a TimeOutError is + raised when timeout number of seconds (or possibly more) expires + without lock acquisition. Raises AlreadyLockedError if the lock is + already set. + + unlock([unconditionally]): + Relinquishes the lock. Raises a NotLockedError if the lock is not + set, unless optional unconditionally is true. + + locked(): + Return true if the lock is set, otherwise false. To avoid race + conditions, this refreshes the lock (on set locks). + + """ + # XXX We need to watch out for two lock objects in the same process + # pointing to the same lock file. Without this, if you lock lf1 and do + # not lock lf2, lf2.locked() will still return true. NOTE: this gimmick + # probably does /not/ work in a multithreaded world, but we don't have to + # worry about that, do we? <1 wink>. + COUNTER = 0 + + def __init__(self, lockfile, lifetime=DEFAULT_LOCK_LIFETIME): + """Create the resource lock using lockfile as the global lock file. + + Each process laying claim to this resource lock will create their own + temporary lock files based on the path specified by lockfile. + Optional lifetime is the number of seconds the process expects to hold + the lock. Optional withlogging, when true, turns on lockfile logging + (see the module docstring for details). + """ + self._lockfile = lockfile + self._lifetime = lifetime + # This works because we know we're single threaded + self._counter = LockFile.COUNTER + LockFile.COUNTER += 1 + self._tmpfname = '%s.%s.%d.%d' % ( + lockfile, socket.gethostname(), os.getpid(), self._counter) + # For transferring ownership across a fork. + self._owned = True + + def __repr__(self): + return '' % ( + id(self), self._lockfile, + self.locked() and 'locked' or 'unlocked', + self._lifetime, os.getpid()) + + def set_lifetime(self, lifetime): + """Set a new lock lifetime. + + This takes affect the next time the file is locked, but does not + refresh a locked file. + """ + self._lifetime = lifetime + + def get_lifetime(self): + """Return the lock's lifetime.""" + return self._lifetime + + def refresh(self, newlifetime=None, unconditionally=False): + """Refreshes the lifetime of a locked file. + + Use this if you realize that you need to keep a resource locked longer + than you thought. With optional newlifetime, set the lock's lifetime. + Raises NotLockedError if the lock is not set, unless optional + unconditionally flag is set to true. + """ + if newlifetime is not None: + self.set_lifetime(newlifetime) + # Do we have the lock? As a side effect, this refreshes the lock! + if not self.locked() and not unconditionally: + raise NotLockedError('%s: %s' % (repr(self), self._read())) + + def lock(self, timeout=0): + """Acquire the lock. + + This blocks until the lock is acquired unless optional timeout is + greater than 0, in which case, a TimeOutError is raised when timeout + number of seconds (or possibly more) expires without lock acquisition. + Raises AlreadyLockedError if the lock is already set. + """ + if timeout: + timeout_time = time.time() + timeout + # Make sure my temp lockfile exists, and that its contents are + # up-to-date (e.g. the temp file name, and the lock lifetime). + self._write() + # XXX This next call can fail with an EPERM. I have no idea why, but + # I'm nervous about wrapping this in a try/except. It seems to be a + # very rare occurence, only happens from cron, and (only?) on Solaris + # 2.6. + self._touch() + log.debug('laying claim: %s', self._lockfile) + # for quieting the logging output + loopcount = -1 + while True: + loopcount += 1 + # Create the hard link and test for exactly 2 links to the file + try: + os.link(self._tmpfname, self._lockfile) + # If we got here, we know we know we got the lock, and never + # had it before, so we're done. Just touch it again for the + # fun of it. + log.debug('got the lock: %s', self._lockfile) + self._touch() + break + except OSError, e: + # The link failed for some reason, possibly because someone + # else already has the lock (i.e. we got an EEXIST), or for + # some other bizarre reason. + if e.errno == errno.ENOENT: + # XXX in some Linux environments, it is possible to get + # an ENOENT, which is truly strange, because this means + # that self._tmpfname doesn't exist at the time of the + # os.link(), but self._write() is supposed to guarantee + # that this happens! I don't honestly know why this + # happens, but for now we just say we didn't acquire the + # lock, and try again next time. + pass + elif e.errno <> errno.EEXIST: + # Something very bizarre happened. Clean up our state and + # pass the error on up. + log.exception('unexpected link') + os.unlink(self._tmpfname) + raise + elif self._linkcount() <> 2: + # Somebody's messin' with us! Log this, and try again + # later. XXX should we raise an exception? + log.error('unexpected linkcount: %d', self._linkcount()) + elif self._read() == self._tmpfname: + # It was us that already had the link. + log.debug('already locked: %s', self._lockfile) + raise AlreadyLockedError + # otherwise, someone else has the lock + pass + # We did not acquire the lock, because someone else already has + # it. Have we timed out in our quest for the lock? + if timeout and timeout_time < time.time(): + os.unlink(self._tmpfname) + log.error('timed out') + raise TimeOutError + # Okay, we haven't timed out, but we didn't get the lock. Let's + # find if the lock lifetime has expired. + if time.time() > self._releasetime() + CLOCK_SLOP: + # Yes, so break the lock. + self._break() + log.error('lifetime has expired, breaking') + # Okay, someone else has the lock, our claim hasn't timed out yet, + # and the expected lock lifetime hasn't expired yet. So let's + # wait a while for the owner of the lock to give it up. + elif not loopcount % 100: + log.debug('waiting for claim: %s', self._lockfile) + self._sleep() + + def unlock(self, unconditionally=False): + """Unlock the lock. + + If we don't already own the lock (either because of unbalanced unlock + calls, or because the lock was stolen out from under us), raise a + NotLockedError, unless optional `unconditionally' is true. + """ + islocked = self.locked() + if not islocked and not unconditionally: + raise NotLockedError + # If we owned the lock, remove the global file, relinquishing it. + if islocked: + try: + os.unlink(self._lockfile) + except OSError, e: + if e.errno <> errno.ENOENT: + raise + # Remove our tempfile + try: + os.unlink(self._tmpfname) + except OSError, e: + if e.errno <> errno.ENOENT: + raise + log.debug('unlocked: %s', self._lockfile) + + def locked(self): + """Return true if we own the lock, false if we do not. + + Checking the status of the lock resets the lock's lifetime, which + helps avoid race conditions during the lock status test. + """ + # Discourage breaking the lock for a while. + try: + self._touch() + except OSError, e: + if e.errno == errno.EPERM: + # We can't touch the file because we're not the owner. I + # don't see how we can own the lock if we're not the owner. + return False + else: + raise + # XXX Can the link count ever be > 2? + if self._linkcount() <> 2: + return False + return self._read() == self._tmpfname + + def finalize(self): + log.debug('finalize: %s', self._lockfile) + self.unlock(unconditionally=True) + + def __del__(self): + log.debug('__del__: %s', self._lockfile) + if self._owned: + self.finalize() + + # Python 2.5 context manager protocol support. + def __enter__(self): + self.lock() + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + self.unlock() + # Don't suppress any exception that might have occurred. + return False + + # Use these only if you're transfering ownership to a child process across + # a fork. Use at your own risk, but it should be race-condition safe. + # _transfer_to() is called in the parent, passing in the pid of the child. + # _take_possession() is called in the child, and blocks until the parent + # has transferred possession to the child. _disown() is used to set the + # _owned flag to false, and it is a disgusting wart necessary to make + # forced lock acquisition work in mailmanctl. :( + def _transfer_to(self, pid): + # First touch it so it won't get broken while we're fiddling about. + self._touch() + # Find out current claim's temp filename + winner = self._read() + # Now twiddle ours to the given pid + self._tmpfname = '%s.%s.%d' % ( + self._lockfile, socket.gethostname(), pid) + # Create a hard link from the global lock file to the temp file. This + # actually does things in reverse order of normal operation because we + # know that lockfile exists, and tmpfname better not! + os.link(self._lockfile, self._tmpfname) + # Now update the lock file to contain a reference to the new owner + self._write() + # Toggle off our ownership of the file so we don't try to finalize it + # in our __del__() + self._owned = False + # Unlink the old winner, completing the transfer + os.unlink(winner) + # And do some sanity checks + assert self._linkcount() == 2 + assert self.locked() + log.debug('transferred the lock: %s', self._lockfile) + + def _take_possession(self): + self._tmpfname = tmpfname = '%s.%s.%d' % ( + self._lockfile, socket.gethostname(), os.getpid()) + # Wait until the linkcount is 2, indicating the parent has completed + # the transfer. + while self._linkcount() <> 2 or self._read() <> tmpfname: + time.sleep(0.25) + log.debug('took possession of the lock: %s', self._lockfile) + + def _disown(self): + self._owned = False + + # + # Private interface + # + + def _write(self): + # Make sure it's group writable + fp = open(self._tmpfname, 'w') + try: + fp.write(self._tmpfname) + finally: + fp.close() + + def _read(self): + try: + fp = open(self._lockfile) + try: + filename = fp.read() + finally: + fp.close() + return filename + except EnvironmentError, e: + if e.errno <> errno.ENOENT: + raise + return None + + def _touch(self, filename=None): + expiration_date = datetime.datetime.now() + self._lifetime + t = time.mktime(expiration_date.timetuple()) + try: + # XXX We probably don't need to modify atime, but this is easier. + os.utime(filename or self._tmpfname, (t, t)) + except OSError, e: + if e.errno <> errno.ENOENT: + raise + + def _releasetime(self): + try: + return os.stat(self._lockfile).st_mtime + except OSError, e: + if e.errno <> errno.ENOENT: + raise + return -1 + + def _linkcount(self): + try: + return os.stat(self._lockfile).st_nlink + except OSError, e: + if e.errno <> errno.ENOENT: + raise + return -1 + + def _break(self): + # First, touch the global lock file. This reduces but does not + # eliminate the chance for a race condition during breaking. Two + # processes could both pass the test for lock expiry in lock() before + # one of them gets to touch the global lockfile. This shouldn't be + # too bad because all they'll do in this function is wax the lock + # files, not claim the lock, and we can be defensive for ENOENTs + # here. + # + # Touching the lock could fail if the process breaking the lock and + # the process that claimed the lock have different owners. We could + # solve this by set-uid'ing the CGI and mail wrappers, but I don't + # think it's that big a problem. + try: + self._touch(self._lockfile) + except OSError, e: + if e.errno <> errno.EPERM: + raise + # Get the name of the old winner's temp file. + winner = self._read() + # Remove the global lockfile, which actually breaks the lock. + try: + os.unlink(self._lockfile) + except OSError, e: + if e.errno <> errno.ENOENT: + raise + # Try to remove the old winner's temp file, since we're assuming the + # winner process has hung or died. Don't worry too much if we can't + # unlink their temp file -- this doesn't wreck the locking algorithm, + # but will leave temp file turds laying around, a minor inconvenience. + try: + if winner: + os.unlink(winner) + except OSError, e: + if e.errno <> errno.ENOENT: + raise + + def _sleep(self): + interval = random.random() * 2.0 + 0.01 + time.sleep(interval) + + + +# Unit test framework +def _dochild(): + prefix = '[%d]' % os.getpid() + # Create somewhere between 1 and 1000 locks + lockfile = LockFile('/tmp/LockTest', lifetime=120) + # Use a lock lifetime of between 1 and 15 seconds. Under normal + # situations, Mailman's usage patterns (untested) shouldn't be much longer + # than this. + workinterval = 5 * random.random() + hitwait = 20 * random.random() + print prefix, 'workinterval:', workinterval + islocked = False + t0 = 0 + t1 = 0 + t2 = 0 + try: + try: + t0 = time.time() + print prefix, 'acquiring...' + lockfile.lock() + print prefix, 'acquired...' + islocked = True + except TimeOutError: + print prefix, 'timed out' + else: + t1 = time.time() + print prefix, 'acquisition time:', t1-t0, 'seconds' + time.sleep(workinterval) + finally: + if islocked: + try: + lockfile.unlock() + t2 = time.time() + print prefix, 'lock hold time:', t2-t1, 'seconds' + except NotLockedError: + print prefix, 'lock was broken' + # wait for next web hit + print prefix, 'webhit sleep:', hitwait + time.sleep(hitwait) + + +def _seed(): + try: + fp = open('/dev/random') + d = fp.read(40) + fp.close() + except EnvironmentError, e: + if e.errno <> errno.ENOENT: + raise + import sha + d = sha.new(`os.getpid()`+`time.time()`).hexdigest() + random.seed(d) + + +def _onetest(): + loopcount = random.randint(1, 100) + for i in range(loopcount): + print 'Loop %d of %d' % (i+1, loopcount) + pid = os.fork() + if pid: + # parent, wait for child to exit + pid, status = os.waitpid(pid, 0) + else: + # child + _seed() + try: + _dochild() + except KeyboardInterrupt: + pass + os._exit(0) + + +def _reap(kids): + if not kids: + return + pid, status = os.waitpid(-1, os.WNOHANG) + if pid <> 0: + del kids[pid] + + +def _test(numtests): + kids = {} + for i in range(numtests): + pid = os.fork() + if pid: + # parent + kids[pid] = pid + else: + # child + _seed() + try: + _onetest() + except KeyboardInterrupt: + pass + os._exit(0) + # slightly randomize each kid's seed + while kids: + _reap(kids) + + +if __name__ == '__main__': + import sys + import random + _test(int(sys.argv[1])) diff --git a/Mailman/queue/archive.py b/Mailman/queue/archive.py index c6565e11d..b0274d49c 100644 --- a/Mailman/queue/archive.py +++ b/Mailman/queue/archive.py @@ -17,11 +17,13 @@ """Archive queue runner.""" +from __future__ import with_statement + import time from email.Utils import parsedate_tz, mktime_tz, formatdate -from Mailman import LockFile from Mailman.configuration import config +from Mailman.lockfile import LockFile from Mailman.queue import Runner @@ -64,14 +66,6 @@ class ArchiveRunner(Runner): msg['X-Original-Date'] = originaldate # Always put an indication of when we received the message. msg['X-List-Received-Date'] = receivedtime - # Now try to get the list lock - try: - mlist.Lock(timeout=config.LIST_LOCK_TIMEOUT) - except LockFile.TimeOutError: - # oh well, try again later - return 1 - try: + # While a list archiving lock is acquired, archive the message. + with LockFile(os.path.join(mlist.full_path, 'archive.lck')): mlist.ArchiveMail(msg) - mlist.Save() - finally: - mlist.Unlock() diff --git a/Mailman/queue/bounce.py b/Mailman/queue/bounce.py index 361b01fac..07fb3ab27 100644 --- a/Mailman/queue/bounce.py +++ b/Mailman/queue/bounce.py @@ -27,7 +27,6 @@ from email.MIMEMessage import MIMEMessage from email.MIMEText import MIMEText from email.Utils import parseaddr -from Mailman import LockFile from Mailman import Utils from Mailman.Bouncers import BouncerAPI from Mailman.Message import UserNotification diff --git a/Mailman/queue/command.py b/Mailman/queue/command.py index a67e757f3..ca53d0192 100644 --- a/Mailman/queue/command.py +++ b/Mailman/queue/command.py @@ -31,7 +31,6 @@ from email.Iterators import typed_subpart_iterator from email.MIMEMessage import MIMEMessage from email.MIMEText import MIMEText -from Mailman import LockFile from Mailman import Message from Mailman import Utils from Mailman.Handlers import Replybot @@ -214,29 +213,18 @@ class CommandRunner(Runner): return False # Now craft the response res = Results(mlist, msg, msgdata) - # BAW: Not all the functions of this qrunner require the list to be - # locked. Still, it's more convenient to lock it here and now and - # deal with lock failures in one place. - try: - mlist.Lock(timeout=config.LIST_LOCK_TIMEOUT) - except LockFile.TimeOutError: - # Oh well, try again later - return True # This message will have been delivered to one of mylist-request, # mylist-join, or mylist-leave, and the message metadata will contain # a key to which one was used. - try: - if msgdata.get('torequest'): - res.process() - elif msgdata.get('tojoin'): - res.do_command('join') - elif msgdata.get('toleave'): - res.do_command('leave') - elif msgdata.get('toconfirm'): - mo = re.match(config.VERP_CONFIRM_REGEXP, msg.get('to', '')) - if mo: - res.do_command('confirm', (mo.group('cookie'),)) - res.send_response() - mlist.Save() - finally: - mlist.Unlock() + if msgdata.get('torequest'): + res.process() + elif msgdata.get('tojoin'): + res.do_command('join') + elif msgdata.get('toleave'): + res.do_command('leave') + elif msgdata.get('toconfirm'): + mo = re.match(config.VERP_CONFIRM_REGEXP, msg.get('to', '')) + if mo: + res.do_command('confirm', (mo.group('cookie'),)) + res.send_response() + config.db.commit() diff --git a/Mailman/queue/incoming.py b/Mailman/queue/incoming.py index 05ab924e6..6118a7ca0 100644 --- a/Mailman/queue/incoming.py +++ b/Mailman/queue/incoming.py @@ -102,7 +102,6 @@ import logging from cStringIO import StringIO from Mailman import Errors -from Mailman import LockFile from Mailman.configuration import config from Mailman.queue import Runner @@ -117,12 +116,6 @@ class IncomingRunner(Runner): def _dispose(self, mlist, msg, msgdata): if msgdata.get('envsender') is None: msg['envsender'] = mlist.no_reply_address - # Try to get the list lock. - try: - mlist.Lock(timeout=config.LIST_LOCK_TIMEOUT) - except LockFile.TimeOutError: - # Oh well, try again later - return 1 # Process the message through a handler pipeline. The handler # pipeline can actually come from one of three places: the message # metadata, the mlist, or the global pipeline. @@ -131,16 +124,13 @@ class IncomingRunner(Runner): # will contain the retry pipeline. Use this above all else. # Otherwise, if the mlist has a `pipeline' attribute, it should be # used. Final fallback is the global pipeline. - try: - pipeline = self._get_pipeline(mlist, msg, msgdata) - msgdata['pipeline'] = pipeline - more = self._dopipeline(mlist, msg, msgdata, pipeline) - if not more: - del msgdata['pipeline'] - mlist.Save() - return more - finally: - mlist.Unlock() + pipeline = self._get_pipeline(mlist, msg, msgdata) + msgdata['pipeline'] = pipeline + more = self._dopipeline(mlist, msg, msgdata, pipeline) + if not more: + del msgdata['pipeline'] + config.db.commit() + return more # Overridable def _get_pipeline(self, mlist, msg, msgdata): diff --git a/Mailman/queue/outgoing.py b/Mailman/queue/outgoing.py index a1e64096d..8322766a4 100644 --- a/Mailman/queue/outgoing.py +++ b/Mailman/queue/outgoing.py @@ -26,7 +26,6 @@ import socket import logging from Mailman import Errors -from Mailman import LockFile from Mailman import Message from Mailman.configuration import config from Mailman.queue import Runner, Switchboard diff --git a/Mailman/tests/test_lockfile.py b/Mailman/tests/test_lockfile.py index ae0753578..9d6420e74 100644 --- a/Mailman/tests/test_lockfile.py +++ b/Mailman/tests/test_lockfile.py @@ -22,7 +22,7 @@ import shutil import tempfile import unittest -from Mailman.LockFile import LockFile +from Mailman.lockfile import LockFile LOCKFILE_NAME = '.mm-test-lock' -- cgit v1.2.3-70-g09d2