Move region configuration to a critical section

Cache initialization performed on the request handling
after Apache wsgi module start raises exception
region.RegionAlreadyConfigured on race condition
in multithreaded mode.

Change-Id: I65f85aedd5b087499b889540417b9502e050ce7c
Closes-bug: 1482271

keystone/tests/unit/test_kvs.py

@@ -27,6 +27,7 @@ from keystone.common.kvs.backends import memcached
 from keystone.common.kvs import core
 from keystone import exception
 from keystone.tests import unit
+from keystone.token.persistence.backends import kvs as token_kvs_backend
 
 
 NO_VALUE = api.NO_VALUE
@@ -583,3 +584,41 @@ class TestMemcachedBackend(unit.TestCase):
         }
         self.assertThat(lambda: memcached.MemcachedBackend(options),
                         raises_valueerror)
+
+
+class TestCacheRegionInit(unit.TestCase):
+    """Illustrate the race condition on cache initialization.
+
+    This case doesn't actually expose the error, it just simulates unprotected
+    code behaviour, when race condition leads to re-configuration of shared
+    KVS backend object. What, in turn, results in an exception.
+    """
+
+    kvs_backend = token_kvs_backend.Token.kvs_backend
+    store_name = 'test-kvs'
+
+    def test_different_instances_initialization(self):
+        """Simulate race condition on token storage initialization."""
+        store = core.get_key_value_store(self.store_name)
+        self.assertFalse(store.is_configured)
+        other_store = core.get_key_value_store(self.store_name)
+        self.assertFalse(other_store.is_configured)
+
+        other_store.configure(backing_store=self.kvs_backend)
+        self.assertTrue(other_store.is_configured)
+        # This error shows that core.get_key_value_store() returns a shared
+        # object protected from re-configuration with an exception.
+        self.assertRaises(RuntimeError, store.configure,
+                          backing_store=self.kvs_backend)
+
+    def test_kvs_configure_called_twice(self):
+        """Check if configure() is called again"""
+        target = core.KeyValueStore
+        with mock.patch.object(target, 'configure') as configure_mock:
+            store = core.get_key_value_store(self.store_name)
+            other_store = core.get_key_value_store(self.store_name)
+
+            store.configure(backing_store=self.kvs_backend)
+            other_store.configure(backing_store=self.kvs_backend)
+
+            self.assertThat(configure_mock.mock_calls, matchers.HasLength(2))

keystone/token/persistence/backends/kvs.py

@@ -15,6 +15,7 @@
 
 from __future__ import absolute_import
 import copy
+import threading
 
 from oslo_config import cfg
 from oslo_log import log
@@ -32,6 +33,8 @@ from keystone.token import provider
 CONF = cfg.CONF
 LOG = log.getLogger(__name__)
 
+STORE_CONF_LOCK = threading.Lock()
+
 
 class Token(token.persistence.TokenDriverV8):
     """KeyValueStore backend for tokens.
@@ -49,9 +52,12 @@ class Token(token.persistence.TokenDriverV8):
         self._store = kvs.get_key_value_store('token-driver')
         if backing_store is not None:
             self.kvs_backend = backing_store
-        if not self._store.is_configured:
-            # Do not re-configure the backend if the store has been initialized
-            self._store.configure(backing_store=self.kvs_backend, **kwargs)
+        # Using a lock here to avoid race condition.
+        with STORE_CONF_LOCK:
+            if not self._store.is_configured:
+                # Do not re-configure the backend if the store has been
+                # initialized.
+                self._store.configure(backing_store=self.kvs_backend, **kwargs)
         if self.__class__ == Token:
             # NOTE(morganfainberg): Only warn if the base KVS implementation
             # is instantiated.