[2.0.x] 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.

Backport of 271542dad1 from master
This commit is contained in:
Michael Sanders 2018-08-01 10:52:28 +01:00 committed by Tim Graham
parent e7dffda167
commit 4441826026
7 changed files with 94 additions and 4 deletions

View File

@ -563,6 +563,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 Thornhill <michael.thornhill@gmail.com>
Michal Chruszcz <troll@pld-linux.org>

View File

@ -502,7 +502,9 @@ class QuerySet:
try:
obj = self.select_for_update().get(**lookup)
except self.model.DoesNotExist:
obj, created = self._create_object_from_params(lookup, 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(lookup, params, lock=True)
if created:
return obj, created
for k, v in defaults.items():
@ -510,7 +512,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().
@ -522,7 +524,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

13
docs/releases/1.11.16.txt Normal file
View File

@ -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`).

13
docs/releases/2.0.9.txt Normal file
View File

@ -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`).

View File

@ -25,6 +25,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
@ -40,6 +41,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

View File

@ -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()

View File

@ -514,6 +514,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(SimpleTestCase):
msg = "Invalid field name(s) for model Thing: 'nonexistent'."