Fixed #29499 -- Fixed race condition in QuerySet.update_or_create().

A race condition happened when the object didn't already exist and
another process/thread created the object before update_or_create()
did and then attempted to update the object, also before update_or_create()
saved the object. The update by the other process/thread could be lost.

AUTHORS

@@ -576,6 +576,7 @@ answer newbie questions, and generally made Django that much better:
     michael.mcewan@gmail.com
     Michael Placentra II <someone@michaelplacentra2.net>
     Michael Radziej <mir@noris.de>
+    Michael Sanders <m.r.sanders@gmail.com>
     Michael Schwarz <michi.schwarz@gmail.com>
     Michael Sinov <sihaelov@gmail.com>
     Michael Thornhill <michael.thornhill@gmail.com>

django/db/models/query.py

@@ -501,7 +501,9 @@ class QuerySet:
                 obj = self.select_for_update().get(**kwargs)
             except self.model.DoesNotExist:
                 params = self._extract_model_params(defaults, **kwargs)
-                obj, created = self._create_object_from_params(kwargs, params)
+                # Lock the row so that a concurrent update is blocked until
+                # after update_or_create() has performed its save.
+                obj, created = self._create_object_from_params(kwargs, params, lock=True)
                 if created:
                     return obj, created
             for k, v in defaults.items():
@@ -509,7 +511,7 @@ class QuerySet:
             obj.save(using=self.db)
         return obj, False
 
-    def _create_object_from_params(self, lookup, params):
+    def _create_object_from_params(self, lookup, params, lock=False):
         """
         Try to create an object using passed params. Used by get_or_create()
         and update_or_create().
@@ -521,7 +523,8 @@ class QuerySet:
             return obj, True
         except IntegrityError as e:
             try:
-                return self.get(**lookup), False
+                qs = self.select_for_update() if lock else self
+                return qs.get(**lookup), False
             except self.model.DoesNotExist:
                 pass
             raise e

docs/releases/1.11.16.txt

@@ -0,0 +1,13 @@
+============================
+Django 1.11.16 release notes
+============================
+
+*Expected September 1, 2018*
+
+Django 1.11.16 fixes a data loss bug in 1.11.15.
+
+Bugfixes
+========
+
+* Fixed a race condition in ``QuerySet.update_or_create()`` that could result
+  in data loss (:ticket:`29499`).

docs/releases/2.0.9.txt

@@ -0,0 +1,13 @@
+==========================
+Django 2.0.9 release notes
+==========================
+
+*Expected September 1, 2018*
+
+Django 2.0.9 fixes a data loss bug in 2.0.8.
+
+Bugfixes
+========
+
+* Fixed a race condition in ``QuerySet.update_or_create()`` that could result
+  in data loss (:ticket:`29499`).

docs/releases/2.1.1.txt

@@ -9,4 +9,5 @@ Django 2.1.1 fixes several bugs in 2.1.
 Bugfixes
 ========
 
-* ...
+* Fixed a race condition in ``QuerySet.update_or_create()`` that could result
+  in data loss (:ticket:`29499`).

docs/releases/index.txt

@@ -40,6 +40,7 @@ versions of the documentation contain the release notes for any later releases.
 .. toctree::
    :maxdepth: 1
 
+   2.0.9
    2.0.8
    2.0.7
    2.0.6
@@ -55,6 +56,7 @@ versions of the documentation contain the release notes for any later releases.
 .. toctree::
    :maxdepth: 1
 
+   1.11.16
    1.11.15
    1.11.14
    1.11.13

tests/get_or_create/models.py

@@ -2,7 +2,7 @@ from django.db import models
 
 
 class Person(models.Model):
-    first_name = models.CharField(max_length=100)
+    first_name = models.CharField(max_length=100, unique=True)
     last_name = models.CharField(max_length=100)
     birthday = models.DateField()
     defaults = models.TextField()

tests/get_or_create/tests.py

@@ -535,6 +535,64 @@ class UpdateOrCreateTransactionTests(TransactionTestCase):
         self.assertGreater(after_update - before_start, timedelta(seconds=0.5))
         self.assertEqual(updated_person.last_name, 'NotLennon')
 
+    @skipUnlessDBFeature('has_select_for_update')
+    @skipUnlessDBFeature('supports_transactions')
+    def test_creation_in_transaction(self):
+        """
+        Objects are selected and updated in a transaction to avoid race
+        conditions. This test checks the behavior of update_or_create() when
+        the object doesn't already exist, but another thread creates the
+        object before update_or_create() does and then attempts to update the
+        object, also before update_or_create(). It forces update_or_create() to
+        hold the lock in another thread for a relatively long time so that it
+        can update while it holds the lock. The updated field isn't a field in
+        'defaults', so update_or_create() shouldn't have an effect on it.
+        """
+        lock_status = {'lock_count': 0}
+
+        def birthday_sleep():
+            lock_status['lock_count'] += 1
+            time.sleep(0.5)
+            return date(1940, 10, 10)
+
+        def update_birthday_slowly():
+            try:
+                Person.objects.update_or_create(first_name='John', defaults={'birthday': birthday_sleep})
+            finally:
+                # Avoid leaking connection for Oracle
+                connection.close()
+
+        def lock_wait(expected_lock_count):
+            # timeout after ~0.5 seconds
+            for i in range(20):
+                time.sleep(0.025)
+                if lock_status['lock_count'] == expected_lock_count:
+                    return True
+            self.skipTest('Database took too long to lock the row')
+
+        # update_or_create in a separate thread.
+        t = Thread(target=update_birthday_slowly)
+        before_start = datetime.now()
+        t.start()
+        lock_wait(1)
+        # Create object *after* initial attempt by update_or_create to get obj
+        # but before creation attempt.
+        Person.objects.create(first_name='John', last_name='Lennon', birthday=date(1940, 10, 9))
+        lock_wait(2)
+        # At this point, the thread is pausing for 0.5 seconds, so now attempt
+        # to modify object before update_or_create() calls save(). This should
+        # be blocked until after the save().
+        Person.objects.filter(first_name='John').update(last_name='NotLennon')
+        after_update = datetime.now()
+        # Wait for thread to finish
+        t.join()
+        # Check call to update_or_create() succeeded and the subsequent
+        # (blocked) call to update().
+        updated_person = Person.objects.get(first_name='John')
+        self.assertEqual(updated_person.birthday, date(1940, 10, 10))  # set by update_or_create()
+        self.assertEqual(updated_person.last_name, 'NotLennon')        # set by update()
+        self.assertGreater(after_update - before_start, timedelta(seconds=1))
+
 
 class InvalidCreateArgumentsTests(TransactionTestCase):
     available_apps = ['get_or_create']