Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#26627 closed Bug (fixed)

outermost atomic block executes on_commit callbacks registered in another outermost atomic block

Reported by: Barthelemy Dagenais Owned by: nobody
Component: Database layer (models, ORM) Version: 1.9
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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 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.

Change History (5)

comment:1 Changed 3 years ago by Barthelemy Dagenais

Has patch: set

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

comment:2 Changed 3 years ago by Simon Charette

Triage Stage: UnreviewedAccepted

comment:3 Changed 3 years ago by Carl Meyer

Triage Stage: AcceptedReady for checkin

comment:4 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In a5c8072a:

Fixed #26627 -- Fixed on_commit callbacks execution order when callbacks make transactions.

comment:5 Changed 3 years ago by Tim Graham <timograham@…>

In 47da073c:

[1.9.x] Fixed #26627 -- Fixed on_commit callbacks execution order when callbacks make transactions.

Backport of a5c8072ab1e56da368a40f3e9c9a3b465f4ffbae from master

Note: See TracTickets for help on using tickets.
Back to Top