Problem/Motivation

Ensure post transaction callbacks are always executed ONLY at the end of the root Drupal transaction.

Proposed resolution

We need to keep track of the very first 'root' transaction object created, because under some circumstances we might end up processing more than one of them during the transaction stack lifecycle.

For example, a root transaction is opened, then a DDL statement is executed in a database that does not support transactional DDL, and another root opened before the original one is closed. By having the first one tracked, we can ensure that post transaction callbacks are processed only upon its destruction and not before.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3440848

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mondrake created an issue. See original summary.

mondrake’s picture

Title: Ensure post transaction callbacks are always executed at the end of the root Drupal transaction » Ensure post transaction callbacks are always executed ONLY at the end of the root Drupal transaction
Priority: Normal » Major
mondrake’s picture

Issue summary: View changes
mondrake’s picture

Issue summary: View changes
mondrake’s picture

Status: Active » Needs review
daffie’s picture

Status: Needs review » Reviewed & tested by the community

All the code changes look good to me.
The added code comments are clear.
For the different situations is testing added.
The testbot is green for all supported databases.
For me it is RTBC.

mondrake’s picture

Issue summary: View changes

quietone made their first commit to this issue’s fork.

quietone’s picture

Status: Reviewed & tested by the community » Needs work

I read the issue summary and the MR. I did not do a code review, just the comments. I applied my own suggestions for minor grammar fixes. The comments on the whole make sense but I suggested some improvements. And I had a question about comments in the tests. I am going to set this to Needs Work so someone checks the suggestions.

I have added credit as well.

mondrake’s picture

Status: Needs work » Needs review

Applied @quietone suggestions, but then changed a bit again as I thought the end result was still not accurate. Not a native speaker, so please bear with me.

quietone’s picture

@mondrake, thanks for updating the comments. They are much easier to understand now and will benefit those who come after us. Thanks!

I was about to restore the RTBC when I checked the tests. There is a failing test, core/tests/Drupal/FunctionalTests/Installer/InstallerTranslationExistingFileTest.php and I have restarted that set of functional tests. Once tests are passing this can go back to RTBC.

mondrake’s picture

Rebased, tests are green now so back to RTBC per #12.

mondrake’s picture

Status: Needs review » Reviewed & tested by the community
quietone’s picture

Title: Ensure post transaction callbacks are always executed ONLY at the end of the root Drupal transaction » Ensure post transaction callbacks are only at the end of the root Drupal transaction
mondrake’s picture

@quietone after your change, the title text no longer reads. Unsure what you meant to change, so leaving as is.

larowlan made their first commit to this issue’s fork.

  • larowlan committed f276f6a9 on 11.x
    Issue #3440848 by mondrake, quietone, daffie: Ensure post transaction...

  • larowlan committed d07ed426 on 11.0.x
    Issue #3440848 by mondrake, quietone, daffie: Ensure post transaction...

  • larowlan committed b54e7f4d on 10.4.x
    Issue #3440848 by mondrake, quietone, daffie: Ensure post transaction...

  • larowlan committed d322b132 on 10.3.x
    Issue #3440848 by mondrake, quietone, daffie: Ensure post transaction...
larowlan’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: +Bug Smash Initiative

Fixed a typo in a comment, waited for a green test-run - which passed.
Committed to 11.x and backported 11.0.x, 10.4.x and 10.3.x

Thanks folks 🙌

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

wim leers’s picture

Wow! What an epic bug and fascinating fix! 👏