Use QuerySet.select_for_update() in admin change form to fix race condition

[Dave Hall]

There is a race condition in the admin change form where a user can edit a model instance at the same time as another user. Consider the following two operations occurring in parallel:

1. ModelAdmin.change_view()
2. User.objects.update(is_superuser=True)

If (1) load the model instance, then (2) runs, then (1) saves the model instance, the update in (2) will be lost.

The solution is to call select_for_update on the queryset in the changeform_view() method of ModelAdmin.

[Tim Graham]

I don't know if this is correct or if it might have some unintended side effects. Is the same issue present in the generic class-based editing views, for example?

[Dave Hall]

Yes, the same issue is also present in the class-based generic edit views.

[Tim Graham]

Can you provide a patch so we can get feedback on it?

[Dave Hall]

Sure, will do. Probably this week.

It's going to be hard to do this in a way that doesn't hurt backwards compatibility. There are a few options we can do:

1. Make the get_object() method take a for_update boolean keyword arg. This will have backwards-compatibility problems.
2. Make the get_object() method call select_for_update() on the queryset if the HTTP method is POST, PUT, PATCH or DELETE. This may result in spurious select_for_update() calls on the queryset, but is backwards compatible.
3. Make the get_queryset() method call select_for_update() on the queryset if the HTTP method is POST, PUT, PATCH or DELETE. This may result in spurious select_for_update() calls on the queryset, but is backwards compatible.

I think that (2) or (3) is the correct approach. I'm erring on the side of (3), since it'll also help with admin bulk actions.

All approaches can be applied to the admin admin generic views. Do people have a preference as to the approach?

[Tim Graham]

If you don't get feedback here, I'd post to the DevelopersMailingList as that reaches a wider audience for design decisions like this.