diff --git a/nova/test.py b/nova/test.py index 54c5dcf8e8..11335d3fad 100644 --- a/nova/test.py +++ b/nova/test.py @@ -96,6 +96,39 @@ class TestingException(Exception): mock_fixture.patch_mock_module() +def _poison_unfair_compute_resource_semaphore_locking(): + """Ensure that every locking on COMPUTE_RESOURCE_SEMAPHORE is called with + fair=True. + """ + orig_synchronized = utils.synchronized + + def poisoned_synchronized(*args, **kwargs): + # Only check fairness if the decorator is used with + # COMPUTE_RESOURCE_SEMAPHORE. But the name of the semaphore can be + # passed as args or as kwargs. + # Note that we cannot import COMPUTE_RESOURCE_SEMAPHORE as that would + # apply the decorators we want to poison here. + if len(args) >= 1: + name = args[0] + else: + name = kwargs.get("name") + if name == "compute_resources" and not kwargs.get("fair", False): + raise AssertionError( + 'Locking on COMPUTE_RESOURCE_SEMAPHORE should always be fair. ' + 'See bug 1864122.') + # go and act like the original decorator + return orig_synchronized(*args, **kwargs) + + # replace the synchronized decorator factory with our own that checks the + # params passed in + utils.synchronized = poisoned_synchronized + + +# NOTE(gibi): This poisoning needs to be done in import time as decorators are +# applied in import time on the ResourceTracker +_poison_unfair_compute_resource_semaphore_locking() + + class NovaExceptionReraiseFormatError(object): real_log_exception = exception.NovaException._log_exception diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py index e2d4457cd0..9af5b5298b 100644 --- a/nova/tests/unit/compute/test_resource_tracker.py +++ b/nova/tests/unit/compute/test_resource_tracker.py @@ -43,6 +43,7 @@ from nova.tests.unit import fake_instance from nova.tests.unit import fake_notifier from nova.tests.unit.objects import test_pci_device as fake_pci_device from nova.tests.unit import utils +from nova import utils as nova_utils from nova.virt import driver _HOSTNAME = 'fake-host' @@ -3896,3 +3897,21 @@ class ResourceTrackerTestCase(test.NoDBTestCase): rt = resource_tracker.ResourceTracker( _HOSTNAME, mock.sentinel.driver, mock.sentinel.reportclient) self.assertIs(rt.reportclient, mock.sentinel.reportclient) + + def test_that_unfair_usage_of_compute_resource_semaphore_is_caught(self): + def _test_explict_unfair(): + class MyResourceTracker(resource_tracker.ResourceTracker): + @nova_utils.synchronized( + resource_tracker.COMPUTE_RESOURCE_SEMAPHORE, fair=False) + def foo(self): + pass + + def _test_implicit_unfair(): + class MyResourceTracker(resource_tracker.ResourceTracker): + @nova_utils.synchronized( + resource_tracker.COMPUTE_RESOURCE_SEMAPHORE) + def foo(self): + pass + + self.assertRaises(AssertionError, _test_explict_unfair) + self.assertRaises(AssertionError, _test_implicit_unfair)