Fixed #26332 -- Fixed a race condition in BaseCache.get_or_set().

3 files changed, 17 insertions, 6 deletions

diff --git a/django/core/cache/backends/base.py b/django/core/cache/backends/base.py
index 75a131ac3a..12351c5bcd 100644
--- a/django/core/cache/backends/base.py
+++ b/django/core/cache/backends/base.py
@@ -154,8 +154,7 @@ class BaseCache(object):
         also be any callable. If timeout is given, that timeout will be used
         for the key; otherwise the default cache timeout will be used.
 
-        Returns the value of the key stored or retrieved on success,
-        False on error.
+        Return the value of the key stored or retrieved.
         """
         if default is None:
             raise ValueError('You need to specify a value.')
@@ -163,9 +162,10 @@ class BaseCache(object):
         if val is None:
             if callable(default):
                 default = default()
-            val = self.add(key, default, timeout=timeout, version=version)
-            if val:
-                return self.get(key, default, version)
+            self.add(key, default, timeout=timeout, version=version)
+            # Fetch the value again to avoid a race condition if another caller
+            # added a value between the first get() and the add() above.
+            return self.get(key, default, version=version)
         return val
 
     def has_key(self, key, version=None):
diff --git a/docs/releases/1.9.5.txt b/docs/releases/1.9.5.txt
index 211cbba6ba..d14b1ac98e 100644
--- a/docs/releases/1.9.5.txt
+++ b/docs/releases/1.9.5.txt
@@ -12,3 +12,7 @@ Bugfixes
 * Made ``MultiPartParser`` ignore filenames that normalize to an empty string
   to fix crash in ``MemoryFileUploadHandler`` on specially crafted user input
   (:ticket:`26325`).
+
+* Fixed a race condition in ``BaseCache.get_or_set()`` (:ticket:`26332`). It
+  now returns the ``default`` value instead of ``False`` if there's an error
+  when trying to add the value to the cache.
diff --git a/tests/cache/tests.py b/tests/cache/tests.py
index e3ae3d83ce..4a3d4bb128 100644
--- a/tests/cache/tests.py
+++ b/tests/cache/tests.py
@@ -30,7 +30,7 @@ from django.template import engines
 from django.template.context_processors import csrf
 from django.template.response import TemplateResponse
 from django.test import (
-    RequestFactory, SimpleTestCase, TestCase, TransactionTestCase,
+    RequestFactory, SimpleTestCase, TestCase, TransactionTestCase, mock,
     override_settings,
 )
 from django.test.signals import setting_changed
@@ -931,6 +931,13 @@ class BaseCacheTests(object):
         self.assertEqual(cache.get_or_set('brian', 1979, version=2), 1979)
         self.assertIsNone(cache.get('brian', version=3))
 
+    def test_get_or_set_racing(self):
+        with mock.patch('%s.%s' % (settings.CACHES['default']['BACKEND'], 'add')) as cache_add:
+            # Simulate cache.add() failing to add a value. In that case, the
+            # default value should be returned.
+            cache_add.return_value = False
+            self.assertEqual(cache.get_or_set('key', 'default'), 'default')
+
 
 @override_settings(CACHES=caches_setting_for_tests(
     BACKEND='django.core.cache.backends.db.DatabaseCache',