#29499: Race condition in QuerySet.update_or_create()
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Sanders |
Type: Bug | Status: new
Component: Database | Version: 2.0
layer (models, ORM) |
Severity: Normal | Keywords: race-condition
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I believe that there is a potential race condition in
`QuerySet.update_or_create()`
When initially trying to obtain the object to update using `get()`, the
row is locked using the `select_for_update()` method - this would lock the
object against changes by other processes until the end of the
transaction. However, if the object does not already exist,
`update_or_create()` then calls the `_create_object_from_params()` method
- see code:
{{{#!python
with transaction.atomic(using=self.db):
try:
obj = self.select_for_update().get(**lookup)
except self.model.DoesNotExist:
obj, created = self._create_object_from_params(lookup,
params)
}}}
The `_create_object_from_params()` method looks like this:
{{{#!python
def _create_object_from_params(self, lookup, params):
"""
Try to create an object using passed params. Used by
get_or_create()
and update_or_create().
"""
try:
with transaction.atomic(using=self.db):
params = {k: v() if callable(v) else v for k, v in
params.items()}
obj = self.create(**params)
return obj, True
except IntegrityError as e:
try:
return self.get(**lookup), False
except self.model.DoesNotExist:
pass
raise e
}}}
Here, it initially tries to create the object. However (assuming that the
object has a unique key constraint policed by the DB), if the object has
already been created meanwhile (by a different process) the
`IntegrityError` is caught and the code uses the `get()` method to attempt
to obtain the newly created object. The object is then returned to the
calling `update_or_create()` method to update - however in this case, the
row has not been locked against changes. So, at this point other processes
are free to modify the DB row and those changes might then be overwritten
by the `save()` in `update_or_create()`.
== Suggested Fix ==
This could be fixed by changing the `_create_object_from_params()` method
to take a new parameter to specify whether the object should be locked on
get, i.e.:
{{{#!python
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().
"""
try:
with transaction.atomic(using=self.db):
params = {k: v() if callable(v) else v for k, v in
params.items()}
obj = self.create(**params)
return obj, True
except IntegrityError as e:
try:
if lock:
return self.select_for_update().get(**lookup), False
else:
return self.get(**lookup), False
except self.model.DoesNotExist:
pass
raise e
}}}
The call to `_create_object_from_params()` from `get_or_create()` would
remain the same, but from `update_or_create()` it would change to:
{{{#!python
obj, created = self._create_object_from_params(lookup,
params, lock=True)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29499>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/055.3116d6d59d8539ce5bb1ab3bf1170f2e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
