Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#33410 closed Bug (fixed)

captureOnCommitCallbacks executes callbacks multiple times.

Reported by: Petter Friberg Owned by: Petter Friberg
Component: Testing framework Version: 4.0
Severity: Release blocker Keywords: captureOnCommitCallbacks
Cc: Eugene Morozov, Adam Johnson 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 (last modified by Petter Friberg)

When recursively adding multiple on commit callbacks, captureOnCommitCallbacks executes some callbacks multiple times. Below is a test case to reproduce the error.

Patch: https://github.com/django/django/pull/15285

  def test_execute_tree(self):
      """
      A visualisation of the callback tree tested. Each node is expected to be visited
      only once. The child count of a node symbolises the amount of on commit
      callbacks.

      root
      └── branch_1
          ├── branch_2
          │   ├── leaf_1
          │   └── leaf_2
          └── leaf_3
      """
      (
          branch_1_call_counter,
          branch_2_call_counter,
          leaf_1_call_counter,
          leaf_2_call_counter,
          leaf_3_call_counter,
      ) = [itertools.count(start=1) for _ in range(5)]

      def leaf_3():
          next(leaf_3_call_counter)

      def leaf_2():
          next(leaf_2_call_counter)

      def leaf_1():
          next(leaf_1_call_counter)

      def branch_2():
          next(branch_2_call_counter)
          transaction.on_commit(leaf_1)
          transaction.on_commit(leaf_2)

      def branch_1():
          next(branch_1_call_counter)
          transaction.on_commit(branch_2)
          transaction.on_commit(leaf_3)

      with self.captureOnCommitCallbacks(execute=True) as callbacks:
          with transaction.atomic():
              transaction.on_commit(branch_1)

      # Every counter shall only have been called once, starting at 1; next value then
      # has to be 2
      self.assertEqual(next(branch_1_call_counter), 2)
      self.assertEqual(next(branch_2_call_counter), 2)
      self.assertEqual(next(leaf_1_call_counter), 2)
      self.assertEqual(next(leaf_2_call_counter), 2)
      self.assertEqual(next(leaf_3_call_counter), 2)
      # Make sure all calls can be seen and the execution order is correct
      self.assertEqual(
          [(id(callback), callback.__name__) for callback in callbacks],
          [
              (id(branch_1), "branch_1"),
              (id(branch_2), "branch_2"),
              (id(leaf_3), "leaf_3"),
              (id(leaf_1), "leaf_1"),
              (id(leaf_2), "leaf_2"),
          ],
      )

Change History (8)

comment:1 by Petter Friberg, 2 years ago

Has patch: set

comment:2 by Petter Friberg, 2 years ago

Description: modified (diff)

comment:3 by Mariusz Felisiak, 2 years ago

Cc: Eugene Morozov Adam Johnson added
Severity: NormalRelease blocker
Summary: captureOnCommitCallbacks executes callbacks multiple timescaptureOnCommitCallbacks executes callbacks multiple times.
Triage Stage: UnreviewedAccepted
Version: dev4.0

Thanks for the report!

Marking as a release blocker since it's a bug in the new feature added in d89f976bddb49fb168334960acc8979c3de991fa (#33054).

comment:4 by Mariusz Felisiak, 2 years ago

Needs documentation: set
Patch needs improvement: set

comment:5 by Petter Friberg, 2 years ago

Needs documentation: unset
Patch needs improvement: unset

comment:6 by Mariusz Felisiak, 2 years ago

Triage Stage: AcceptedReady for checkin

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In bc174e6:

Fixed #33410 -- Fixed recursive capturing of callbacks by TestCase.captureOnCommitCallbacks().

Regression in d89f976bddb49fb168334960acc8979c3de991fa.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In 11475958:

[4.0.x] Fixed #33410 -- Fixed recursive capturing of callbacks by TestCase.captureOnCommitCallbacks().

Regression in d89f976bddb49fb168334960acc8979c3de991fa.

Backport of bc174e6ea0ce676c5a7f467bda9739e6ef6b6186 from main

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