Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mitigate problems with EntityManager::flush() reentrance since 2.16.0 #10906

Closed
wants to merge 2 commits into from
Closed

Mitigate problems with EntityManager::flush() reentrance since 2.16.0 #10906

wants to merge 2 commits into from

Commits on Aug 17, 2023

  1. Mitigate problems with EntityManager::flush() reentrance since 2.16.0 

    This PR addresses the issue brought up in doctrine#10869. It happens when users use event listeners – `postPersist` in particular – to make changes to entities and/or persist new entities and finally call `EntityManager::flush()`, while the `UnitOfWork` is currently within the commit phase.

There is a discussion in doctrine#10900 whether this kind of reentrance should be deprecated in 2.x and prevented in 3.x. But, in order to prevent complete breakage with the 2.16.0 update, this PR tries to apply a band-aid 🩹.

A few things changed inside the UoW commit implementation in 2.16, and for sure this PR does not restore all details of the previous behavior. Take it with a grain of salt.

Here's the details.

The issue appears when `UoW::commit()` is performing entity insertions, and `postPersist` listener causes `commit()` reentrance when there are pending insertions left.

In that situation, the "inner" (reentrant) call will start working through all changesets. Eventually, it finishes with all insertions being performed and `UoW::$entityInsertions` being empty.

The entity insertion order, an array computed at the beginning of `UnitOfWork::executeInserts()`, still contains entities that now have been processed already. This leads to the reported failure down the road. The mitigation is to check for this condition and skip such entities.

Before doctrine#10547 (pre-2.16), things worked a bit differently:

The UoW did not build a list of entities (objects) as the commit order, but a sequence of _classes_ to process.

For every entity _class_, it would find all entity instances in `UoW::$entityInsertions` and pass them to the persister in a batch. The persister, in turn, would execute the `INSERT` statements for _all_ those entities _before_ it dispatched the `postInsert` events.

That means when a `postInsert` listener was notified pre-2.16, the UoW was in a state where 1) all insertions for a particular class had been written to the database already and 2) the UoW had not yet gathered the next round of entities to process.
    @mpdude
    mpdude committed Aug 17, 2023
    Configuration menu
    Copy the full SHA
    1895cf8 View commit details
    Browse the repository at this point in the history

  2. Defer dispatching postInsert events until all inserts have been exe… 

    …cuted
    @mpdude
    mpdude committed Aug 17, 2023
    Configuration menu
    Copy the full SHA
    b66c3b4 View commit details
    Browse the repository at this point in the history