Using remove in preFlush event results in spl_object_hash conflict #6344

Closed
opened 2026-01-22 15:31:25 +01:00 by admin · 13 comments
Owner

Originally created by @develancer on GitHub (Nov 18, 2019).

Originally assigned to: @beberlei on GitHub.

Bug Report

Q A
BC Break no
Version 2.6.4

Summary

We have had a number of reported bugs related to spl_object_hash conflicts in the past, and it seems the problem is not eliminated yet. For example, EntityManager's (and UnitOfWork's) remove method 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:

A managed+dirty entity TestEntity@000000001d515bca000000005d0092d6 can not be scheduled for insertion.

How to reproduce

Entity specification:

/**
 * @Table(name="test_table")
 * @Entity
 * @HasLifecycleCallbacks
 */
class TestEntity
{
	/**
	 * @Id
	 * @Column(name="id", type="integer", nullable=false)
	 */
	private $id;

	/**
	 * @Column(name="value", type="integer", nullable=false)
	 */
	private $value = 0;

	public function __construct($id)
	{
		$this->id = (int) $id;
	}

	/** @PreFlush */
	public function preFlush(PreFlushEventArgs $args)
	{
		// prevents empty records from storing in the database
		if (!$this->value) {
			$args->getEntityManager()->remove($this);
		}
	}
}

Core of the unit test:

public function runTestForPreFlush(EntityManagerInterface $em)
{
	$this->createEmptyEntity($em, 1);
	$em->flush();
	$this->createEmptyEntity($em, 2);
	$em->flush();
}

private function createEmptyEntity(EntityManagerInterface $em, $id)
{
	$entity = new TestEntity($id);
	$em->persist($entity);
}

Expected behavior

Both flush operations should perform without error, and none of the entities should be saved into the database.

Originally created by @develancer on GitHub (Nov 18, 2019). Originally assigned to: @beberlei on GitHub. ### Bug Report | Q | A |------------ | ------ | BC Break | no | Version | 2.6.4 #### Summary We have had a number of reported bugs related to `spl_object_hash` conflicts in the past, and it seems the problem is not eliminated yet. For example, EntityManager's (and UnitOfWork's) `remove` method 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: > A managed+dirty entity TestEntity@000000001d515bca000000005d0092d6 can not be scheduled for insertion. #### How to reproduce Entity specification: ```php /** * @Table(name="test_table") * @Entity * @HasLifecycleCallbacks */ class TestEntity { /** * @Id * @Column(name="id", type="integer", nullable=false) */ private $id; /** * @Column(name="value", type="integer", nullable=false) */ private $value = 0; public function __construct($id) { $this->id = (int) $id; } /** @PreFlush */ public function preFlush(PreFlushEventArgs $args) { // prevents empty records from storing in the database if (!$this->value) { $args->getEntityManager()->remove($this); } } } ``` Core of the unit test: ```php public function runTestForPreFlush(EntityManagerInterface $em) { $this->createEmptyEntity($em, 1); $em->flush(); $this->createEmptyEntity($em, 2); $em->flush(); } private function createEmptyEntity(EntityManagerInterface $em, $id) { $entity = new TestEntity($id); $em->persist($entity); } ``` #### Expected behavior Both flush operations should perform without error, and none of the entities should be saved into the database.
admin added the BugInvalid labels 2026-01-22 15:31:25 +01:00
admin closed this issue 2026-01-22 15:31:25 +01:00
Author
Owner

@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?

@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?
Author
Owner

@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.

@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.
Author
Owner

@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?

@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?
Author
Owner

@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 flush itself should not be called inside the preFlush event. Therefore, we should either:

  • fix this bug, or
  • update the documentation and state that remove cannot be safely called within preFlush event listeners.
@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 `flush` itself should not be called inside the `preFlush` event. Therefore, we should either: * fix this bug, or * update the documentation and state that `remove` cannot be safely called within `preFlush` event listeners.
Author
Owner

@Ocramius commented on GitHub (Nov 22, 2019):

Does moving this to onFlush change the behavior at all? I remember DoctrineExtensions doing something like that for soft-deletes...

@Ocramius commented on GitHub (Nov 22, 2019): Does moving this to `onFlush` change the behavior at all? I remember DoctrineExtensions doing something like that for soft-deletes...
Author
Owner

@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?

@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?
Author
Owner

@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 as docker run docker-example.

If you need an interactive PHP shell for debugging, docker run -it docker-example -a will do.

@develancer commented on GitHub (Nov 28, 2019): @piotrkardasz Attached you'll find a dockerized test case [docker-example.zip](https://github.com/doctrine/orm/files/3901707/docker-example.zip). You can build it with `docker build -t docker-example .` and then run it as `docker run docker-example`. If you need an interactive PHP shell for debugging, `docker run -it docker-example -a` will do.
Author
Owner

@develancer commented on GitHub (Nov 28, 2019):

@Ocramius onFlush also fails — see attached docker-example-onflush.zip.

@develancer commented on GitHub (Nov 28, 2019): @Ocramius `onFlush` also fails — see attached [docker-example-onflush.zip](https://github.com/doctrine/orm/files/3901851/docker-example-onflush.zip).
Author
Owner

@piotrkardasz commented on GitHub (Nov 30, 2019):

@develancer You should call method flush once.

e.g.

for ($i = 0; $i <10; $i++)
{
createEmptyEntity($em, $i);
}
$em->flush();

@piotrkardasz commented on GitHub (Nov 30, 2019): @develancer You should call method `flush` once. e.g. > for ($i = 0; $i <10; $i++) > { > createEmptyEntity($em, $i); > } > $em->flush();
Author
Owner

@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 flush once.” ;-)

@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 `flush` once.” ;-)
Author
Owner

@beberlei commented on GitHub (Dec 1, 2019):

This happens because you call remove while computeChangeSet is in progress. The removal however doesn't stop that operation, and data is then entered into originalEntityData and 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 preFlush instead of a lifecycle event.

class PreventInsertListener
{
    public function onPreFlush($args)
    {
        $entityManager = $args->geTentityManager();
        foreach ($entityManager->getUnitOfWork()->getScheduledEntityInsertions() as $entity) {
             $entityManager->remove($entity);
        }
    }
}

This happens slightly earlier and will work.

@beberlei commented on GitHub (Dec 1, 2019): This happens because you call remove while `computeChangeSet` is in progress. The removal however doesn't stop that operation, and data is then entered into `originalEntityData` and 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 `preFlush` instead of a lifecycle event. ```php class PreventInsertListener { public function onPreFlush($args) { $entityManager = $args->geTentityManager(); foreach ($entityManager->getUnitOfWork()->getScheduledEntityInsertions() as $entity) { $entityManager->remove($entity); } } } ``` This happens slightly earlier and will work.
Author
Owner

@develancer commented on GitHub (Dec 1, 2019):

@beberlei Shouldn't we mention it in the docs, then?

@develancer commented on GitHub (Dec 1, 2019): @beberlei Shouldn't we mention it in the docs, then?
Author
Owner

@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() from preFlush listener. I know it's forbidden in docs, but we ended up with the flush after some major refactoring.

@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()` from `preFlush` listener. I know it's forbidden in docs, but we ended up with the flush after some major refactoring.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6344