diff --git a/lib/locking.py b/lib/locking.py index 783ec542bc646b7bf8ed1cbffa111b6c73cd3ce4..fd84fe663d3087272fad8eb0cb18fbc29650c583 100644 --- a/lib/locking.py +++ b/lib/locking.py @@ -560,9 +560,9 @@ class LockSet: shared: is the pre-acquisition shared? """ - - assert not self.__lock._is_owned(shared=1), ( - "Cannot add new elements while sharing the set-lock") + # Check we don't already own locks at this level + assert not self._is_owned() or self.__lock._is_owned(shared=0), \ + "Cannot add locks if the set is only partially owned, or shared" # Support passing in a single resource to add rather than many if isinstance(names, basestring): diff --git a/test/ganeti.locking_unittest.py b/test/ganeti.locking_unittest.py index a43e1b7896f9062bc5883cc6eb202b6ae9ed8f73..94843fecfc34404df9e599bcd094c12e3a96c82a 100755 --- a/test/ganeti.locking_unittest.py +++ b/test/ganeti.locking_unittest.py @@ -354,12 +354,20 @@ class TestLockSet(unittest.TestCase): self.assert_('five' not in self.ls._names()) self.assert_('six' not in self.ls._names()) self.assertEquals(self.ls._list_owned(), set(['seven'])) - self.ls.add('eight', acquired=1, shared=1) - self.assert_('eight' in self.ls._names()) - self.assertEquals(self.ls._list_owned(), set(['seven', 'eight'])) + self.assertRaises(AssertionError, self.ls.add, 'eight', acquired=1) self.ls.remove('seven') self.assert_('seven' not in self.ls._names()) - self.assertEquals(self.ls._list_owned(), set(['eight'])) + self.assertEquals(self.ls._list_owned(), set([])) + self.ls.acquire(None, shared=1) + self.assertRaises(AssertionError, self.ls.add, 'eight') + self.ls.release() + self.ls.acquire(None) + self.ls.add('eight', acquired=1) + self.assert_('eight' in self.ls._names()) + self.assert_('eight' in self.ls._list_owned()) + self.ls.add('nine') + self.assert_('nine' in self.ls._names()) + self.assert_('nine' not in self.ls._list_owned()) self.ls.release() self.ls.remove(['two']) self.assert_('two' not in self.ls._names()) @@ -550,6 +558,32 @@ class TestLockSet(unittest.TestCase): self.assertEqual(self.done.get(True, 1), 'DONE') self.assertEqual(self.done.get(True, 1), 'DONE') + def testConcurrentSetLockAdd(self): + self.ls.acquire('one') + # Another thread wants the whole SetLock + Thread(target=self._doLockSet, args=(None, 0)).start() + Thread(target=self._doLockSet, args=(None, 1)).start() + self.assertRaises(Queue.Empty, self.done.get, True, 0.2) + self.assertRaises(AssertionError, self.ls.add, 'four') + self.ls.release() + self.assertEqual(self.done.get(True, 1), 'DONE') + self.assertEqual(self.done.get(True, 1), 'DONE') + self.ls.acquire(None) + Thread(target=self._doLockSet, args=(None, 0)).start() + Thread(target=self._doLockSet, args=(None, 1)).start() + self.assertRaises(Queue.Empty, self.done.get, True, 0.2) + self.ls.add('four') + self.ls.add('five', acquired=1) + self.ls.add('six', acquired=1, shared=1) + self.assertEquals(self.ls._list_owned(), + set(['one', 'two', 'three', 'five', 'six'])) + self.assertEquals(self.ls._is_owned(), True) + self.assertEquals(self.ls._names(), + set(['one', 'two', 'three', 'four', 'five', 'six'])) + self.ls.release() + self.assertEqual(self.done.get(True, 1), 'DONE') + self.assertEqual(self.done.get(True, 1), 'DONE') + def testEmptyLockSet(self): # get the set-lock self.assertEqual(self.ls.acquire(None), set(['one', 'two', 'three']))