diff options
author | Julien Danjou <julien@danjou.info> | 2014-10-02 17:35:25 +0200 |
---|---|---|
committer | Julien Danjou <julien@danjou.info> | 2014-10-03 11:09:14 +0200 |
commit | 4dd034690f9bd1b3fe3bef30cb40f70c06a3fb8e (patch) | |
tree | a10ee6cfce2780a449d3893f5e2c9916b2aeadae | |
parent | aabe3c5e67cee64f55e21ed5b4d98696e0c7decd (diff) | |
download | tooz-4dd034690f9bd1b3fe3bef30cb40f70c06a3fb8e.tar.gz |
coordination: remove destroy() from the lock protocol
This is remove the need to manually destroy the lock, especially in the
IPC driver. It's very complicated to know when we can destroy a lock, so
what we do is that we destroy it when releasing, and make the rest of
the code solid so it handles lock being destroyed under its feet.
Change-Id: I67eab4e403b0d3194cf8b082a3059ef4536c3988
-rw-r--r-- | tooz/drivers/ipc.py | 77 | ||||
-rw-r--r-- | tooz/locking.py | 3 | ||||
-rw-r--r-- | tooz/tests/test_coordination.py | 3 |
3 files changed, 30 insertions, 53 deletions
diff --git a/tooz/drivers/ipc.py b/tooz/drivers/ipc.py index 5184ac7..2c5727a 100644 --- a/tooz/drivers/ipc.py +++ b/tooz/drivers/ipc.py @@ -45,24 +45,8 @@ class IPCLock(locking.Lock): def __init__(self, name, timeout): super(IPCLock, self).__init__(name) self.key = self.ftok(name, self._LOCK_PROJECT) - while True: - try: - # Try to create the lock - self._lock = sysv_ipc.Semaphore(self.key, - flags=sysv_ipc.IPC_CREX, - initial_value=1) - except sysv_ipc.ExistentialError: - # We failed to create it because it already exists, then try to - # grab the existing one. - try: - self._lock = sysv_ipc.Semaphore(self.key) - except sysv_ipc.ExistentialError: - # Semaphore has been deleted in the mean time, retry from - # the beginning! - pass - break - self._lock.undo = True self.timeout = timeout + self._lock = None @staticmethod def ftok(name, project): @@ -83,39 +67,38 @@ class IPCLock(locking.Lock): and sysv_ipc.SEMAPHORE_TIMEOUT_SUPPORTED is False): raise tooz.NotImplemented( "This system does not support semaphore timeout") - try: - self._lock.acquire(timeout=timeout) - except (sysv_ipc.BusyError, sysv_ipc.ExistentialError): - return False - else: - return True + while True: + try: + self._lock = sysv_ipc.Semaphore(self.key, + flags=sysv_ipc.IPC_CREX, + initial_value=1) + self._lock.undo = True + except sysv_ipc.ExistentialError: + # We failed to create it because it already exists, then try to + # grab the existing one. + try: + self._lock = sysv_ipc.Semaphore(self.key) + self._lock.undo = True + except sysv_ipc.ExistentialError: + # Semaphore has been deleted in the mean time, retry from + # the beginning! + pass + try: + self._lock.acquire(timeout=timeout) + except sysv_ipc.BusyError: + return False + except sysv_ipc.ExistentialError: + # Likely the lock has been deleted in the meantime, retry + pass + else: + return True def release(self): - try: - self._lock.release() - except sysv_ipc.ExistentialError: - return False - else: - return True - - def destroy(self): - """This will destroy the lock. - - NOTE(harlowja): this will destroy the lock, and if it is being shared - across processes this can have unintended consquences, so it *must* - only be used when it is *safe* to remove it (ie at a known program - exit point, where it can be ensured that no other process will be - using it, or that if other processes are using it they can tolerate - it being destroyed). - - Read your man pages for `semctl(IPC_RMID)` before using this to - understand its side-effects on other programs that *may* be - concurrently using the same lock while it is being destroyed... - """ - try: + if self._lock is not None: self._lock.remove() - except sysv_ipc.ExistentialError: - pass + self._lock = None + return True + return False class IPCDriver(coordination.CoordinationDriver): diff --git a/tooz/locking.py b/tooz/locking.py index b12912e..27b193a 100644 --- a/tooz/locking.py +++ b/tooz/locking.py @@ -50,6 +50,3 @@ class Lock(object): :returns: returns true if acquired (false if not) :rtype: bool """ - - def destroy(self): - """Removes the lock + any resources associated with the lock.""" diff --git a/tooz/tests/test_coordination.py b/tooz/tests/test_coordination.py index d974908..7b119e9 100644 --- a/tooz/tests/test_coordination.py +++ b/tooz/tests/test_coordination.py @@ -437,7 +437,6 @@ class TestAPI(testscenarios.TestWithScenarios, def test_get_lock(self): lock = self._coord.get_lock(self._get_random_uuid()) - self.addCleanup(lock.destroy) self.assertEqual(True, lock.acquire()) lock.release() with lock: @@ -451,11 +450,9 @@ class TestAPI(testscenarios.TestWithScenarios, lock_name = self._get_random_uuid() lock = self._coord.get_lock(lock_name) - self.addCleanup(lock.destroy) self.assertEqual(True, lock.acquire()) lock2 = client2.get_lock(lock_name) - self.addCleanup(lock2.destroy) self.assertEqual(False, lock2.acquire(blocking=False)) lock.release() self.assertEqual(True, lock2.acquire(blocking=True)) |