4

How can I update a model field based on its current value and avoid a race condition? The update task can be written as:

if (self.x == y):
    self.x = z
    self.save()
else:
    raise Exception()

However, there is a race condition. I came up with the following solution:

from django.db import transaction
with transaction.atomic():
    if (self.x == y):
        self.x = z
        self.save()
    else:
        raise Exception()

But is this safe and is there a better way?

Kevin Christopher Henry
  • 46,175
  • 7
  • 116
  • 102
Jaakko Luttinen
  • 695
  • 2
  • 7
  • 21

1 Answers1

7

No, that atomic() block won't do anything because you've already fetched the values from the database and into self before you try and run the transaction.

If you can express your conditional in query arguments you could do this safely in a single query using update():

num_matched = MyModel.objects.filter(id=self.id, x=y).update(x=z)
if num_matched != 1:
    raise Exception()

If not, you can use select_for_update():

with transaction.atomic():
    current = MyModel.objects.select_for_update().get(id=self.id)
    if (current.x == y):
        current.x = z
        current.save()
    else:
        raise Exception()

The difference between this and your code above is that here you're explicitly telling the database what row to lock before doing your comparison.

Kevin Christopher Henry
  • 46,175
  • 7
  • 116
  • 102
  • First, I was wondering why you need to use `transaction.atomic` because `select_for_update` should lock the row. However, it locks only within the contex of a single transaction and each query will be its own transaction (if using autocommit), as explained [here](http://stackoverflow.com/a/17149748/2184571). Therefore, you need the `transaction.atomic` wrapping, right? – Jaakko Luttinen Oct 19 '15 at 07:17
  • 1
    @jluttine: Yes, exactly. From the documentation: "Evaluating a queryset with `select_for_update()` in autocommit mode... is a `TransactionManagementError` error because the rows are not locked in that case." – Kevin Christopher Henry Oct 19 '15 at 07:34
  • Actually, a few questions: First, isn't `transaction.atomic` unnecessary because the only transaction after `get` is `save` and no other transactions? Second, is the `select_for_update` lock released if raising the exception? In that case, no transaction is generated so the lock isn't unreleased, or is it? – Jaakko Luttinen Oct 19 '15 at 08:57
  • 3
    @jluttine: To really understand this you will have to read about database transactions. But briefly: 1) the `get()` and `save()` are two different operations that need to be in a transaction; by definition, autocommit puts a single operation in a transaction. 2) the `atomic()` context manager will respond to the raised exception by telling the database to roll back the transaction, which releases the lock. – Kevin Christopher Henry Oct 19 '15 at 09:46