Commit 3f404fc5 authored by Guido Trotter's avatar Guido Trotter
Browse files

LockSet: improve remove() api

Lockset's remove() function used to return a list of locks we failed to remove.
Rather than doing this we'll return a list of removed locks, so it's more
similar to how acquire() behaves. This patch also fixes the relevant unit tests.

Reviewed-by: imsnah
parent 0cc00929
......@@ -512,8 +512,8 @@ class LockSet:
you are already holding exclusively the locks.
Returns:
A list of lock which we failed to delete. The list is always empty if we
were holding all the locks exclusively.
A list of lock which we removed. The list is always equal to the names
list if we were holding all the locks exclusively.
"""
if not blocking and not self._is_owned():
......@@ -530,7 +530,7 @@ class LockSet:
assert not self._is_owned() or self._list_owned().issuperset(names), (
"remove() on acquired lockset while not owning all elements")
delete_failed=[]
removed = []
for lname in names:
# Calling delete() acquires the lock exclusively if we don't already own
......@@ -540,8 +540,8 @@ class LockSet:
# everything we want to delete, or we hold none.
try:
self.__lockdict[lname].delete()
removed.append(lname)
except (KeyError, errors.LockError):
delete_failed.append(lname)
# This cannot happen if we were already holding it, verify:
assert not self._is_owned(), "remove failed while holding lockset"
else:
......@@ -557,7 +557,7 @@ class LockSet:
if self._is_owned():
self._del_owned(lname)
return delete_failed
return removed
# Locking levels, must be acquired in increasing order.
......
......@@ -291,7 +291,7 @@ class TestLockSet(unittest.TestCase):
self.assert_('six' in self.ls._names())
self.assert_('seven' in self.ls._names())
self.assertEquals(self.ls._list_owned(), set(['five', 'six', 'seven']))
self.ls.remove(['five', 'six'])
self.assertEquals(self.ls.remove(['five', 'six']), ['five', 'six'])
self.assert_('five' not in self.ls._names())
self.assert_('six' not in self.ls._names())
self.assertEquals(self.ls._list_owned(), set(['seven']))
......@@ -305,18 +305,19 @@ class TestLockSet(unittest.TestCase):
self.ls.remove(['two'])
self.assert_('two' not in self.ls._names())
self.ls.acquire('three')
self.ls.remove(['three'])
self.assertEquals(self.ls.remove(['three']), ['three'])
self.assert_('three' not in self.ls._names())
self.assertEquals(self.ls.remove('three'), ['three'])
self.assertEquals(self.ls.remove(['one', 'three', 'six']), ['three', 'six'])
self.assertEquals(self.ls.remove('three'), [])
self.assertEquals(self.ls.remove(['one', 'three', 'six']), ['one'])
self.assert_('one' not in self.ls._names())
def testRemoveNonBlocking(self):
self.assertRaises(NotImplementedError, self.ls.remove, 'one', blocking=0)
self.ls.acquire('one')
self.assertEquals(self.ls.remove('one', blocking=0), [])
self.assertEquals(self.ls.remove('one', blocking=0), ['one'])
self.ls.acquire(['two', 'three'])
self.assertEquals(self.ls.remove(['two', 'three'], blocking=0), [])
self.assertEquals(self.ls.remove(['two', 'three'], blocking=0),
['two', 'three'])
def testNoDoubleAdd(self):
self.assertRaises(errors.LockError, self.ls.add, 'two')
......@@ -406,9 +407,9 @@ class TestLockSet(unittest.TestCase):
Thread(target=self._doRemoveSet, args=(['four', 'six'], )).start()
self.assertRaises(Queue.Empty, self.done.get, True, 0.2)
self.ls.remove('four')
self.assertEqual(self.done.get(True, 1), ['four'])
self.assertEqual(self.done.get(True, 1), ['six'])
Thread(target=self._doRemoveSet, args=(['two'])).start()
self.assertEqual(self.done.get(True, 1), [])
self.assertEqual(self.done.get(True, 1), ['two'])
self.ls.release()
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment