From patchwork Tue Oct 20 12:08:42 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Lespiau X-Patchwork-Id: 532932 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id BB1541402B2 for ; Tue, 20 Oct 2015 23:09:42 +1100 (AEDT) Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 9546C1A0AD5 for ; Tue, 20 Oct 2015 23:09:42 +1100 (AEDT) X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by lists.ozlabs.org (Postfix) with ESMTP id 4A9A41A0304 for ; Tue, 20 Oct 2015 23:09:24 +1100 (AEDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga101.fm.intel.com with ESMTP; 20 Oct 2015 05:09:04 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,707,1437462000"; d="scan'208";a="667966945" Received: from dwoodhou-linux.ger.corp.intel.com (HELO strange.ger.corp.intel.com) ([10.252.9.158]) by orsmga003.jf.intel.com with ESMTP; 20 Oct 2015 05:08:45 -0700 From: Damien Lespiau To: patchwork@lists.ozlabs.org Subject: [PATCH 1/2] lock: Import file lock class from mercurial Date: Tue, 20 Oct 2015 13:08:42 +0100 Message-Id: <1445342923-6310-2-git-send-email-damien.lespiau@intel.com> X-Mailer: git-send-email 2.4.3 In-Reply-To: <1445342923-6310-1-git-send-email-damien.lespiau@intel.com> References: <1445342923-6310-1-git-send-email-damien.lespiau@intel.com> X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" With series now in production, I realized I needed the mail parsing to be atomic. Because of the race can happen between mutiple process a file lock seems like it could work. Signed-off-by: Damien Lespiau --- patchwork/lock.py | 301 +++++++++++++++++++++++++++++++++++++++++++ patchwork/tests/test_lock.py | 290 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 591 insertions(+) create mode 100644 patchwork/lock.py create mode 100644 patchwork/tests/test_lock.py diff --git a/patchwork/lock.py b/patchwork/lock.py new file mode 100644 index 0000000..41ee972 --- /dev/null +++ b/patchwork/lock.py @@ -0,0 +1,301 @@ +# lock.py - simple advisory locking scheme for mercurial +# +# Copyright 2005, 2006 Matt Mackall +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2 or any later version. +# + +# This file has been taken from the mercurial project, the base revision it's +# derived from is: +# https://selenic.com/hg/rev/e8564e04382d +# A few changes have been made: +# - revert to not using the vfs object +# - import a few functions and classes from util.py and error.py + +from __future__ import absolute_import + +import contextlib +import errno +import os +import socket +import time +import warnings + +# +# from error.py +# + +class LockError(IOError): + def __init__(self, errno, strerror, filename, desc): + IOError.__init__(self, errno, strerror, filename) + self.desc = desc + +class LockHeld(LockError): + def __init__(self, errno, filename, desc, locker): + LockError.__init__(self, errno, 'Lock held', filename, desc) + self.locker = locker + +class LockUnavailable(LockError): + pass + +# LockError is for errors while acquiring the lock -- this is unrelated +class LockInheritanceContractViolation(RuntimeError): + pass + +# +# from util.py +# + +def testpid(pid): + '''return False if pid dead, True if running or not sure''' + if os.sys.platform == 'OpenVMS': + return True + try: + os.kill(pid, 0) + return True + except OSError as inst: + return inst.errno != errno.ESRCH + +def makelock(info, pathname): + try: + return os.symlink(info, pathname) + except OSError as why: + if why.errno == errno.EEXIST: + raise + except AttributeError: # no symlink in os + pass + + ld = os.open(pathname, os.O_CREAT | os.O_WRONLY | os.O_EXCL) + os.write(ld, info) + os.close(ld) + +def readlock(pathname): + try: + return os.readlink(pathname) + except OSError as why: + if why.errno not in (errno.EINVAL, errno.ENOSYS): + raise + except AttributeError: # no symlink in os + pass + fp = posixfile(pathname) + r = fp.read() + fp.close() + return r + +# +# from lock.py +# + +class lock(object): + '''An advisory lock held by one process to control access to a set + of files. Non-cooperating processes or incorrectly written scripts + can ignore Mercurial's locking scheme and stomp all over the + repository, so don't do that. + + Typically used via localrepository.lock() to lock the repository + store (.hg/store/) or localrepository.wlock() to lock everything + else under .hg/.''' + + # lock is symlink on platforms that support it, file on others. + + # symlink is used because create of directory entry and contents + # are atomic even over nfs. + + # old-style lock: symlink to pid + # new-style lock: symlink to hostname:pid + + _host = None + + def __init__(self, file, timeout=-1, releasefn=None, acquirefn=None, + desc=None, inheritchecker=None, parentlock=None): + self.f = file + self.held = 0 + self.timeout = timeout + self.releasefn = releasefn + self.acquirefn = acquirefn + self.desc = desc + self._inheritchecker = inheritchecker + self.parentlock = parentlock + self._parentheld = False + self._inherited = False + self.postrelease = [] + self.pid = self._getpid() + self.delay = self.lock() + if self.acquirefn: + self.acquirefn() + + def __del__(self): + if self.held: + warnings.warn("use lock.release instead of del lock", + category=DeprecationWarning, + stacklevel=2) + + # ensure the lock will be removed + # even if recursive locking did occur + self.held = 1 + + self.release() + + def _getpid(self): + # wrapper around os.getpid() to make testing easier + return os.getpid() + + def lock(self): + timeout = self.timeout + while True: + try: + self._trylock() + return self.timeout - timeout + except LockHeld as inst: + if timeout != 0: + time.sleep(1) + if timeout > 0: + timeout -= 1 + continue + raise LockHeld(errno.ETIMEDOUT, inst.filename, self.desc, + inst.locker) + + def _trylock(self): + if self.held: + self.held += 1 + return + if lock._host is None: + lock._host = socket.gethostname() + lockname = '%s:%s' % (lock._host, self.pid) + retry = 5 + while not self.held and retry: + retry -= 1 + try: + makelock(lockname, self.f) + self.held = 1 + except (OSError, IOError) as why: + if why.errno == errno.EEXIST: + locker = self._readlock() + # special case where a parent process holds the lock -- this + # is different from the pid being different because we do + # want the unlock and postrelease functions to be called, + # but the lockfile to not be removed. + if locker == self.parentlock: + self._parentheld = True + self.held = 1 + return + locker = self._testlock(locker) + if locker is not None: + raise LockHeld(errno.EAGAIN, self.f, self.desc, locker) + else: + raise LockUnavailable(why.errno, why.strerror, + why.filename, self.desc) + + def _readlock(self): + """read lock and return its value + + Returns None if no lock exists, pid for old-style locks, and host:pid + for new-style locks. + """ + try: + return readlock(self.f) + except (OSError, IOError) as why: + if why.errno == errno.ENOENT: + return None + raise + + def _testlock(self, locker): + if locker is None: + return None + try: + host, pid = locker.split(":", 1) + except ValueError: + return locker + if host != lock._host: + return locker + try: + pid = int(pid) + except ValueError: + return locker + if testpid(pid): + return locker + # if locker dead, break lock. must do this with another lock + # held, or can race and break valid lock. + try: + l = lock(self.f + '.break', timeout=0) + os.unlink(self.f) + l.release() + except LockError: + return locker + + def testlock(self): + """return id of locker if lock is valid, else None. + + If old-style lock, we cannot tell what machine locker is on. + with new-style lock, if locker is on this machine, we can + see if locker is alive. If locker is on this machine but + not alive, we can safely break lock. + + The lock file is only deleted when None is returned. + + """ + locker = self._readlock() + return self._testlock(locker) + + @contextlib.contextmanager + def inherit(self): + """context for the lock to be inherited by a Mercurial subprocess. + + Yields a string that will be recognized by the lock in the subprocess. + Communicating this string to the subprocess needs to be done separately + -- typically by an environment variable. + """ + if not self.held: + raise LockInheritanceContractViolation( + 'inherit can only be called while lock is held') + if self._inherited: + raise error.LockInheritanceContractViolation( + 'inherit cannot be called while lock is already inherited') + if self._inheritchecker is not None: + self._inheritchecker() + if self.releasefn: + self.releasefn() + if self._parentheld: + lockname = self.parentlock + else: + lockname = '%s:%s' % (lock._host, self.pid) + self._inherited = True + try: + yield lockname + finally: + if self.acquirefn: + self.acquirefn() + self._inherited = False + + def release(self): + """release the lock and execute callback function if any + + If the lock has been acquired multiple times, the actual release is + delayed to the last release call.""" + if self.held > 1: + self.held -= 1 + elif self.held == 1: + self.held = 0 + if self._getpid() != self.pid: + # we forked, and are not the parent + return + try: + if self.releasefn: + self.releasefn() + finally: + if not self._parentheld: + try: + os.unlink(self.f) + except OSError: + pass + # The postrelease functions typically assume the lock is not held + # at all. + if not self._parentheld: + for callback in self.postrelease: + callback() + +def release(*locks): + for lock in locks: + if lock is not None: + lock.release() diff --git a/patchwork/tests/test_lock.py b/patchwork/tests/test_lock.py new file mode 100644 index 0000000..cd05f7f --- /dev/null +++ b/patchwork/tests/test_lock.py @@ -0,0 +1,290 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2015 Intel Corporation +# +# This file is part of the Patchwork package. +# +# Patchwork 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. +# +# Patchwork 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 Patchwork; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +# This file is import from mercurial (tests/test-lock.py) with a few +# modifications to work with patchwork's lock.py version. +# +# The last revision it was synced with is: +# https://selenic.com/hg/rev/e72b62b154b0 + +from __future__ import absolute_import + +import copy +import os +import tempfile +import types +import unittest + +from patchwork import lock +from patchwork import lock as error + +testlockname = 'testlock' + +# work around http://bugs.python.org/issue1515 +if types.MethodType not in copy._deepcopy_dispatch: + def _deepcopy_method(x, memo): + return type(x)(x.im_func, copy.deepcopy(x.im_self, memo), x.im_class) + copy._deepcopy_dispatch[types.MethodType] = _deepcopy_method + +class lockwrapper(lock.lock): + def __init__(self, pidoffset, *args, **kwargs): + # lock.lock.__init__() calls lock(), so the pidoffset assignment needs + # to be earlier + self._pidoffset = pidoffset + super(lockwrapper, self).__init__(*args, **kwargs) + def _getpid(self): + return os.getpid() + self._pidoffset + +class teststate(object): + def __init__(self, testcase, dir, pidoffset=0): + self._testcase = testcase + self._acquirecalled = False + self._releasecalled = False + self._postreleasecalled = False + self._pidoffset = pidoffset + + def makelock(self, *args, **kwargs): + l = lockwrapper(self._pidoffset, testlockname, releasefn=self.releasefn, + acquirefn=self.acquirefn, *args, **kwargs) + l.postrelease.append(self.postreleasefn) + return l + + def acquirefn(self): + self._acquirecalled = True + + def releasefn(self): + self._releasecalled = True + + def postreleasefn(self): + self._postreleasecalled = True + + def assertacquirecalled(self, called): + self._testcase.assertEqual( + self._acquirecalled, called, + 'expected acquire to be %s but was actually %s' % ( + self._tocalled(called), + self._tocalled(self._acquirecalled), + )) + + def resetacquirefn(self): + self._acquirecalled = False + + def assertreleasecalled(self, called): + self._testcase.assertEqual( + self._releasecalled, called, + 'expected release to be %s but was actually %s' % ( + self._tocalled(called), + self._tocalled(self._releasecalled), + )) + + def assertpostreleasecalled(self, called): + self._testcase.assertEqual( + self._postreleasecalled, called, + 'expected postrelease to be %s but was actually %s' % ( + self._tocalled(called), + self._tocalled(self._postreleasecalled), + )) + + def assertlockexists(self, exists): + actual = os.path.lexists(testlockname) + self._testcase.assertEqual( + actual, exists, + 'expected lock to %s but actually did %s' % ( + self._toexists(exists), + self._toexists(actual), + )) + + def _tocalled(self, called): + if called: + return 'called' + else: + return 'not called' + + def _toexists(self, exists): + if exists: + return 'exist' + else: + return 'not exist' + +class testlock(unittest.TestCase): + def testlock(self): + state = teststate(self, tempfile.mkdtemp(dir=os.getcwd())) + lock = state.makelock() + state.assertacquirecalled(True) + lock.release() + state.assertreleasecalled(True) + state.assertpostreleasecalled(True) + state.assertlockexists(False) + + def testrecursivelock(self): + state = teststate(self, tempfile.mkdtemp(dir=os.getcwd())) + lock = state.makelock() + state.assertacquirecalled(True) + + state.resetacquirefn() + lock.lock() + # recursive lock should not call acquirefn again + state.assertacquirecalled(False) + + lock.release() # brings lock refcount down from 2 to 1 + state.assertreleasecalled(False) + state.assertpostreleasecalled(False) + state.assertlockexists(True) + + lock.release() # releases the lock + state.assertreleasecalled(True) + state.assertpostreleasecalled(True) + state.assertlockexists(False) + + def testlockfork(self): + state = teststate(self, tempfile.mkdtemp(dir=os.getcwd())) + lock = state.makelock() + state.assertacquirecalled(True) + + # fake a fork + forklock = copy.deepcopy(lock) + forklock._pidoffset = 1 + forklock.release() + state.assertreleasecalled(False) + state.assertpostreleasecalled(False) + state.assertlockexists(True) + + # release the actual lock + lock.release() + state.assertreleasecalled(True) + state.assertpostreleasecalled(True) + state.assertlockexists(False) + + def testinheritlock(self): + d = tempfile.mkdtemp(dir=os.getcwd()) + parentstate = teststate(self, d) + parentlock = parentstate.makelock() + parentstate.assertacquirecalled(True) + + # set up lock inheritance + with parentlock.inherit() as lockname: + parentstate.assertreleasecalled(True) + parentstate.assertpostreleasecalled(False) + parentstate.assertlockexists(True) + + childstate = teststate(self, d, pidoffset=1) + childlock = childstate.makelock(parentlock=lockname) + childstate.assertacquirecalled(True) + + childlock.release() + childstate.assertreleasecalled(True) + childstate.assertpostreleasecalled(False) + childstate.assertlockexists(True) + + parentstate.resetacquirefn() + + parentstate.assertacquirecalled(True) + + parentlock.release() + parentstate.assertreleasecalled(True) + parentstate.assertpostreleasecalled(True) + parentstate.assertlockexists(False) + + def testmultilock(self): + d = tempfile.mkdtemp(dir=os.getcwd()) + state0 = teststate(self, d) + lock0 = state0.makelock() + state0.assertacquirecalled(True) + + with lock0.inherit() as lock0name: + state0.assertreleasecalled(True) + state0.assertpostreleasecalled(False) + state0.assertlockexists(True) + + state1 = teststate(self, d, pidoffset=1) + lock1 = state1.makelock(parentlock=lock0name) + state1.assertacquirecalled(True) + + # from within lock1, acquire another lock + with lock1.inherit() as lock1name: + # since the file on disk is lock0's this should have the same + # name + self.assertEqual(lock0name, lock1name) + + state2 = teststate(self, d, pidoffset=2) + lock2 = state2.makelock(parentlock=lock1name) + state2.assertacquirecalled(True) + + lock2.release() + state2.assertreleasecalled(True) + state2.assertpostreleasecalled(False) + state2.assertlockexists(True) + + state1.resetacquirefn() + + state1.assertacquirecalled(True) + + lock1.release() + state1.assertreleasecalled(True) + state1.assertpostreleasecalled(False) + state1.assertlockexists(True) + + lock0.release() + + def testinheritlockfork(self): + d = tempfile.mkdtemp(dir=os.getcwd()) + parentstate = teststate(self, d) + parentlock = parentstate.makelock() + parentstate.assertacquirecalled(True) + + # set up lock inheritance + with parentlock.inherit() as lockname: + childstate = teststate(self, d, pidoffset=1) + childlock = childstate.makelock(parentlock=lockname) + childstate.assertacquirecalled(True) + + # fork the child lock + forkchildlock = copy.deepcopy(childlock) + forkchildlock._pidoffset += 1 + forkchildlock.release() + childstate.assertreleasecalled(False) + childstate.assertpostreleasecalled(False) + childstate.assertlockexists(True) + + # release the child lock + childlock.release() + childstate.assertreleasecalled(True) + childstate.assertpostreleasecalled(False) + childstate.assertlockexists(True) + + parentlock.release() + + def testinheritcheck(self): + d = tempfile.mkdtemp(dir=os.getcwd()) + state = teststate(self, d) + def check(): + raise error.LockInheritanceContractViolation('check failed') + lock = state.makelock(inheritchecker=check) + state.assertacquirecalled(True) + + def tryinherit(): + with lock.inherit(): + pass + + self.assertRaises(error.LockInheritanceContractViolation, tryinherit) + + lock.release() + +if __name__ == '__main__': + unittest.main(__name__)