[Django] #26627: outermost atomic block executes on_commit callbacks registered in another outermost atomic block

29 views
Skip to first unread message

Django

unread,
May 17, 2016, 10:52:56 AM5/17/16
to django-...@googlegroups.com
#26627: outermost atomic block executes on_commit callbacks registered in another
outermost atomic block
----------------------------------------------+--------------------
Reporter: bartdag | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
The on_commit hook executes callbacks in an undesired order because the
list of callbacks is tied to the connection instead of being tied to the
transaction or atomic block. Consider the following code:

{{{
from django.db import transaction
from foobar.models import Model1, Model2

@transaction.atomic
def outer():
print("START - OUTER")
for i in range(4):
f1(i)
print("END - OUTER")

@transaction.atomic
def f1(i):
model = Model1(description="test {0}".format(i))
model.save()
transaction.on_commit(lambda: commit_hook(model, i))

def commit_hook(model, i):
print("START - on_commit hook")
f2(i)
print("STOP - on_commit hook")

@transaction.atomic
def f2(i):
print("CALLING F2")
model = Model2(description="test {0}".format(i))
model.save()
}}}

The expected trace when calling outer() is:
{{{
START - OUTER
END - OUTER
START - on_commit hook
CALLING F2
STOP - on_commit hook
START - on_commit hook
CALLING F2
STOP - on_commit hook
START - on_commit hook
CALLING F2
STOP - on_commit hook
START - on_commit hook
CALLING F2
STOP - on_commit hook
}}}

But the actual trace is:
{{{
START - OUTER
END - OUTER
START - on_commit hook
CALLING F2
START - on_commit hook
CALLING F2
START - on_commit hook
CALLING F2
START - on_commit hook
CALLING F2
STOP - on_commit hook
STOP - on_commit hook
STOP - on_commit hook
STOP - on_commit hook
}}}

Essentially, the outermost atomic block on f2() is executing the callbacks
registered by the outermost atomic block on outer(), which creates a stack
of calls instead of a sequence of calls.

This has been discussed on the [https://groups.google.com/forum/#!topic
/django-users/e2lbYGqKszo Django user list] and more examples of why this
can lead to undesired behaviors are given there.

Carl Meyer has suggested a fix. I'll provide a regression test and a fix
today.

--
Ticket URL: <https://code.djangoproject.com/ticket/26627>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
May 17, 2016, 10:59:43 AM5/17/16
to django-...@googlegroups.com
#26627: outermost atomic block executes on_commit callbacks registered in another
outermost atomic block
-------------------------------------+-------------------------------------

Reporter: bartdag | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by bartdag):

* needs_better_patch: => 0
* has_patch: 0 => 1
* needs_tests: => 0
* needs_docs: => 0


Comment:

I submitted a pull request based on Carl Meyer's suggestion:
https://github.com/django/django/pull/6617

--
Ticket URL: <https://code.djangoproject.com/ticket/26627#comment:1>

Django

unread,
May 17, 2016, 11:24:26 AM5/17/16
to django-...@googlegroups.com
#26627: outermost atomic block executes on_commit callbacks registered in another
outermost atomic block
-------------------------------------+-------------------------------------

Reporter: bartdag | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by charettes):

* stage: Unreviewed => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/26627#comment:2>

Django

unread,
May 18, 2016, 2:42:14 AM5/18/16
to django-...@googlegroups.com
#26627: outermost atomic block executes on_commit callbacks registered in another
outermost atomic block
-------------------------------------+-------------------------------------

Reporter: bartdag | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by carljm):

* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/26627#comment:3>

Django

unread,
May 18, 2016, 9:10:47 AM5/18/16
to django-...@googlegroups.com
#26627: outermost atomic block executes on_commit callbacks registered in another
outermost atomic block
-------------------------------------+-------------------------------------
Reporter: bartdag | Owner: nobody
Type: Bug | Status: closed

Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

* status: new => closed
* resolution: => fixed


Comment:

In [changeset:"a5c8072ab1e56da368a40f3e9c9a3b465f4ffbae" a5c8072a]:
{{{
#!CommitTicketReference repository=""
revision="a5c8072ab1e56da368a40f3e9c9a3b465f4ffbae"
Fixed #26627 -- Fixed on_commit callbacks execution order when callbacks
make transactions.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26627#comment:4>

Django

unread,
May 18, 2016, 9:16:56 AM5/18/16
to django-...@googlegroups.com
#26627: outermost atomic block executes on_commit callbacks registered in another
outermost atomic block
-------------------------------------+-------------------------------------
Reporter: bartdag | Owner: nobody

Type: Bug | Status: closed
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"47da073cb01f86f6f71cf19a2866f80a9755f434" 47da073c]:
{{{
#!CommitTicketReference repository=""
revision="47da073cb01f86f6f71cf19a2866f80a9755f434"
[1.9.x] Fixed #26627 -- Fixed on_commit callbacks execution order when
callbacks make transactions.

Backport of a5c8072ab1e56da368a40f3e9c9a3b465f4ffbae from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26627#comment:5>

Reply all
Reply to author
Forward
0 new messages