[PR #10900] RFC: Prevent UnitOfWork::commit() reentrance? #12686

New Issue

admin · 2026-01-22T16:14:51+01:00

admin commented

2026-01-22 16:14:51 +01:00

Original Pull Request: https://github.com/doctrine/orm/pull/10900

State: closed
Merged: No

This PR prevents reentrant calls to UnitOfWork::commit() with a clear exception message.

Reentrance happens, for example, when users call EntityManager::flush() from within postPersist or similar event listeners. Since #10547, it causes strange-looking, non-helpful error messages (#10869).

Instead of treating this as a bug and trying to fix it, my suggestion is to fail with a clear exception message instead.

Reasons:

I assume the UoW was never designed to support reentrant flush() calls in the first place. So, trying to make (or keep) this working might be a rabbit hole.

The assumption is based on the following:

Not a single test in the ORM test suite covers or even triggers flush() reentrance – otherwise, such a test would have failed with the changes suggested here.
Documentation for e. g. preFlush or postFlush explicitly mentions that flush() must not be called again. I don't know why this is not also stated for e. g. postPersist. But why would it be safe to call flush() during postPersist, in the middle of a transaction, when there are reasons against calling it in postFlush, just before final cleanups are made?
Last but not least, entity insertions, computed changesets etc. are kept in fields like UnitOfWork::$entityChangeSets or UnitOfWork::$originalEntityData. It's all to easy to mess up these states when we re-compute changesets mid-way through a flush() – and again, if that were anticipated, I'd assume to find any kind of test coverage.

TODO

Review #10906 – revert that change, maybe in 3.0?

**Original Pull Request:** https://github.com/doctrine/orm/pull/10900 **State:** closed **Merged:** No --- This PR prevents reentrant calls to `UnitOfWork::commit()` with a clear exception message. Reentrance happens, for example, when users call `EntityManager::flush()` from within `postPersist` or similar event listeners. Since #10547, it causes strange-looking, non-helpful error messages (#10869). Instead of treating this as a bug and trying to fix it, my suggestion is to fail with a clear exception message instead. Reasons: I assume the UoW was never designed to support reentrant `flush()` calls in the first place. So, trying to make (or keep) this working might be a rabbit hole. The assumption is based on the following: * Not a single test in the ORM test suite covers or even triggers `flush()` reentrance – otherwise, such a test would have failed with the changes suggested here. * Documentation for e. g. [`preFlush`](https://www.doctrine-project.org/projects/doctrine-orm/en/2.16/reference/events.html#preflush) or [`postFlush`](https://www.doctrine-project.org/projects/doctrine-orm/en/2.16/reference/events.html#preflush) explicitly mentions that `flush()` must not be called again. I don't know why this is not also stated for e. g. `postPersist`. But why would it be safe to call `flush()` during `postPersist`, in the middle of a transaction, when there are reasons against calling it in `postFlush`, just before final cleanups are made? * Last but not least, entity insertions, computed changesets etc. are kept in fields like `UnitOfWork::$entityChangeSets` or `UnitOfWork::$originalEntityData`. It's all to easy to mess up these states when we re-compute changesets mid-way through a `flush()` – and again, if that were anticipated, I'd assume to find any kind of test coverage. #### TODO - [ ] Review #10906 – revert that change, maybe in 3.0?

admin added the pull-request label 2026-01-22 16:14:51 +01:00

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: doctrine/archived-orm#12686

[PR #10900] RFC: Prevent UnitOfWork::commit() reentrance? #12686

TODO

[PR #10900] RFC: Prevent `UnitOfWork::commit()` reentrance? #12686