id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
29499	Race condition in QuerySet.update_or_create()	Michael Sanders	Michael Sanders	"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)
}}}"	Bug	closed	Database layer (models, ORM)	dev	Normal	fixed	race-condition		Ready for checkin	1	0	0	0	0	0