#20571: Using savepoints within transaction.atomic() can result in the entire
transaction being incorrectly and silently rolled back
----------------------------------------------+--------------------
Reporter: lamby | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Using savepoints within transaction.atomic() can result in the entire
transaction being incorrectly and silently rolled back. Here is a slightly
contrived example:
{{{#!python
from django.db import transaction, DatabaseError
with transaction.atomic():
user = User.objects.all()[0]
user.last_login = datetime.datetime.utcnow() # any change to
demonstrate problem
user.save()
sid = transaction.savepoint()
try:
# Will always fail
User.objects.create(pk=user.pk)
transaction.savepoint_commit(sid)
except DatabaseError:
transaction.savepoint_rollback(sid)
# outside of atomic() context - user.last_login change has not been
committed!
}}}
What happens is the .create() call fails and marks the outermost atomic
block as requiring a rollback, so even though we call savepoint_rollback
(which does exactly right thing), once we leave the outer
transaction.atomic(), the entire transaction is rolled back. In the
example above, this means that the change to last_login is not persisted
in the database. Rather insiduously, it is done entirely silently. :)
(The outer transaction.atomic() seems a little contrived but note that it
is equivalent to the wrapper added by ATOMIC_REQUESTS.)
I'm not entirely sure what the solution is; the
transaction.atomic(savepoint=False) call within ModelBase.save_base simply
does not (and cannot) know that some other code will manually handle the
savepoint rollback and thus no choice but to mark the entire transaction
as borked. The only way it could know is if .create and .save took yet
another kwarg, but this seems bizarre and would not be at all obvious.
Maybe manual savepoints should be become a private API after all
(contradicting the current note in the documentation). Alternatively, it
could be clarified that "manual" savepoint management they should not be
used in conjunction with atomic() blocks (and thus ATOMIC_REQUESTS). The
behaviour above is certainly not obvious at all.
(Hm, update_or_create uses savepoints manually, I hope that isn't breaking
installations?)
--
Ticket URL: <https://code.djangoproject.com/ticket/20571>
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/048.fa2ae1777066b77aa1cb489ad965c1b1%40djangoproject.com?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
