mirror of
https://github.com/doctrine/orm.git
synced 2026-03-24 06:52:09 +01:00
Using remove in preFlush event results in spl_object_hash conflict #6344
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @develancer on GitHub (Nov 18, 2019).
Originally assigned to: @beberlei on GitHub.
Bug Report
Summary
We have had a number of reported bugs related to
spl_object_hashconflicts in the past, and it seems the problem is not eliminated yet. For example, EntityManager's (and UnitOfWork's)removemethod cannot be safely used in PreFlush.Current behavior
If the
remove()is called on an entity within a PreFlush event, it is garbage collected but its hash is still remembered by the EntityManager. Creating another entity afterwards (with a subsequent flush) can result in hash conflict and ORM error:How to reproduce
Entity specification:
Core of the unit test:
Expected behavior
Both flush operations should perform without error, and none of the entities should be saved into the database.
@SenseException commented on GitHub (Nov 19, 2019):
"Expected behavior" misses more detailed description. Do you expect that none of the entities in the example will be saved into the database? What is your use case?
@develancer commented on GitHub (Nov 19, 2019):
@SenseException Exactly. I have just fixed the description, thanks. However, the main problem here is the “managed+dirty” error.
The use case is that for each base record we have a number of optional additional records in auxiliary tables, and if any of such records does not exist, all fields are assumed to have default values. Therefore, if a user changes all fields in any given record back to defaults, the record doesn't have to exist anymore and can be deleted to save space.
@piotrkardasz commented on GitHub (Nov 22, 2019):
In my opinion is mishmash logic. If EntityManager is step before flush you would like to flush and remove in the same instance. Why you don't check the value property before persist?
@develancer commented on GitHub (Nov 22, 2019):
@piotrkardasz It may as well be, but the real question is: is it allowed by the documentation? In my opinion, it is, as the documentation states only that
flushitself should not be called inside thepreFlushevent. Therefore, we should either:removecannot be safely called withinpreFlushevent listeners.@Ocramius commented on GitHub (Nov 22, 2019):
Does moving this to
onFlushchange the behavior at all? I remember DoctrineExtensions doing something like that for soft-deletes...@piotrkardasz commented on GitHub (Nov 23, 2019):
I created the same model as you presented and it work correctly as you described in Expected behavior Could you explain how to reproduce your case?
@develancer commented on GitHub (Nov 28, 2019):
@piotrkardasz Attached you'll find a dockerized test case docker-example.zip. You can build it with
docker build -t docker-example .and then run it asdocker run docker-example.If you need an interactive PHP shell for debugging,
docker run -it docker-example -awill do.@develancer commented on GitHub (Nov 28, 2019):
@Ocramius
onFlushalso fails — see attached docker-example-onflush.zip.@piotrkardasz commented on GitHub (Nov 30, 2019):
@develancer You should call method
flushonce.e.g.
@develancer commented on GitHub (Dec 1, 2019):
@piotrkardasz :) I hope you understand that's clearly not the point. Most of the complicated database workflows will utilize multiple small transactions instead of a single large one.
This workflow is correct and it adheres to the docs. Unless you find some part of the documentation that states “You should call method
flushonce.” ;-)@beberlei commented on GitHub (Dec 1, 2019):
This happens because you call remove while
computeChangeSetis in progress. The removal however doesn't stop that operation, and data is then entered intooriginalEntityDataand other stateful properties inside the UnitOfWork, causing this problem.I believe this is not fixable, calling remove() at this point will not work. You need to do this in an EventListener that listens to
preFlushinstead of a lifecycle event.This happens slightly earlier and will work.
@develancer commented on GitHub (Dec 1, 2019):
@beberlei Shouldn't we mention it in the docs, then?
@biozshock commented on GitHub (Dec 4, 2019):
For those who landed here from search. After update from 2.6.3 to 2.7.0 the ORM throws this exception when you issue
flush()frompreFlushlistener. I know it's forbidden in docs, but we ended up with the flush after some major refactoring.