DDC-2332: Ensure managed entities are always tracked by UOW

doctrine · Jan 12, 2022 · 481e12e · 481e12e
1 parent ca32fc6
commit 481e12e
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 42 deletions.
diff --git a/lib/Doctrine/ORM/Tools/SchemaValidator.php b/lib/Doctrine/ORM/Tools/SchemaValidator.php
@@ -239,7 +239,7 @@ public function validateClass(ClassMetadataInfo $class)
             }
         }
 
-        if (! $class->isInheritanceTypeNone() && ! $class->isRootEntity() && ! $class->isMappedSuperclass && array_search($class->name, $class->discriminatorMap, true) === false) {
+        if (! $class->isInheritanceTypeNone() && ! $class->isRootEntity() && array_search($class->name, $class->discriminatorMap, true) === false) {
             $ce[] = "Entity class '" . $class->name . "' is part of inheritance hierarchy, but is " .
                 "not mapped in the root entity '" . $class->rootEntityName . "' discriminator map. " .
                 'All subclasses must be listed in the discriminator map.';

diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
@@ -303,6 +303,15 @@ class UnitOfWork implements PropertyChangedListener
     /** @var ReflectionPropertiesGetter */
     private $reflectionPropertiesGetter;
 
+    /**
+     * Associate entities with OIDs to ensure the GC won't recycle a managed entity
+     *
+     * DDC-2332 / #3037
+     *
+     * @psalm-var array<int, object>
+     */
+    private $oidMap = [];
+
     /**
      * Initializes a new UnitOfWork instance, bound to the given EntityManager.
      */
@@ -1532,7 +1541,8 @@ public function isEntityScheduled($entity)
     public function addToIdentityMap($entity)
     {
         $classMetadata = $this->em->getClassMetadata(get_class($entity));
-        $identifier    = $this->entityIdentifiers[spl_object_id($entity)];
+        $oid           = spl_object_id($entity);
+        $identifier    = $this->entityIdentifiers[$oid];
 
         if (empty($identifier) || in_array(null, $identifier, true)) {
             throw ORMInvalidArgumentException::entityWithoutIdentity($classMetadata->name, $entity);
@@ -1541,6 +1551,8 @@ public function addToIdentityMap($entity)
         $idHash    = implode(' ', $identifier);
         $className = $classMetadata->rootEntityName;
 
+        $this->oidMap[$oid] = $entity;
+
         if (isset($this->identityMap[$className][$idHash])) {
             return false;
         }
@@ -1655,6 +1667,7 @@ public function removeFromIdentityMap($entity)
         }
 
         $className = $classMetadata->rootEntityName;
+        unset($this->oidMap[$oid]);
 
         if (isset($this->identityMap[$className][$idHash])) {
             unset($this->identityMap[$className][$idHash], $this->readOnlyObjects[$oid]);
@@ -2526,6 +2539,7 @@ public function clear($entityName = null)
     {
         if ($entityName === null) {
             $this->identityMap                    =
+            $this->oidMap                         =
             $this->entityIdentifiers              =
             $this->originalEntityData             =
             $this->entityChangeSets               =
@@ -2700,6 +2714,7 @@ public function createEntity($className, array $data, &$hints = [])
             $this->entityIdentifiers[$oid]  = $id;
             $this->entityStates[$oid]       = self::STATE_MANAGED;
             $this->originalEntityData[$oid] = $data;
+            $this->oidMap[$oid]             = $entity;
 
             $this->identityMap[$class->rootEntityName][$idHash] = $entity;
 

diff --git a/tests/Doctrine/Tests/ORM/Functional/IdentityMapTest.php b/tests/Doctrine/Tests/ORM/Functional/IdentityMapTest.php
@@ -6,11 +6,13 @@
 
 use Doctrine\ORM\Query;
 use Doctrine\Tests\Models\CMS\CmsAddress;
+use Doctrine\Tests\Models\CMS\CmsArticle;
 use Doctrine\Tests\Models\CMS\CmsPhonenumber;
 use Doctrine\Tests\Models\CMS\CmsUser;
 use Doctrine\Tests\OrmFunctionalTestCase;
 
 use function count;
+use function gc_collect_cycles;
 use function get_class;
 
 /**
@@ -257,4 +259,48 @@ public function testCollectionValuedAssociationIdentityMapBehaviorWithRefresh():
         // Now the collection should be refreshed with correct count
         self::assertCount(4, $user2->getPhonenumbers());
     }
+
+    /**
+     * @group HashCollision
+     */
+    public function testHashCollision(): void
+    {
+        $user           = new CmsUser();
+        $user->username = 'Test';
+        $user->name     = 'Test';
+        $this->_em->persist($user);
+        $this->_em->flush();
+
+        $articles = [];
+        for ($i = 0; $i < 100; ++$i) {
+            $articles[$i]   = $article = new CmsArticle();
+            $article->topic = 'Test';
+            $article->text  = 'Test';
+            $article->setAuthor($this->_em->merge($user));
+            $this->_em->persist($article);
+            $this->_em->flush();
+            $this->_em->clear();
+        }
+
+        $user = $this->_em->merge($user);
+
+        foreach ($articles as $article) {
+            $article = $this->_em->merge($article);
+            $article->setAuthor($user);
+        }
+
+        unset($article);
+        gc_collect_cycles();
+        $keep = [];
+
+        for ($x = 0; $x < 1000; ++$x) {
+            $keep[$x]       = $article = new CmsArticle();
+            $article->topic = 'Test';
+            $article->text  = 'Test';
+            $article->setAuthor($this->_em->merge($user));
+            $this->_em->persist($article);
+            $this->_em->flush();
+            $this->assertNotNull($article->id, 'Article wasn\'t persisted on iteration ' . $x);
+        }
+    }
 }
diff --git a/tests/Doctrine/Tests/ORM/Tools/SchemaValidatorTest.php b/tests/Doctrine/Tests/ORM/Tools/SchemaValidatorTest.php
@@ -211,46 +211,6 @@ public function testInvalidAssociationInsideEmbeddable(): void
             $ce
         );
     }
-
-    /**
-     * @group 8771
-     */
-    public function testMappedSuperclassNotPresentInDiscriminator(): void
-    {
-        $class1 = $this->em->getClassMetadata(MappedSuperclass::class);
-        $ce     = $this->validator->validateClass($class1);
-
-        $this->assertEquals([], $ce);
-    }
-}
-
-/**
- * @MappedSuperClass
- */
-abstract class MappedSuperclass extends ParentEntity
-{
-}
-
-/**
- * @Entity
- * @InheritanceType("SINGLE_TABLE")
- * @DiscriminatorMap({"child" = ChildEntity::class})
- */
-abstract class ParentEntity
-{
-    /**
-     * @var mixed
-     * @Id
-     * @Column
-     */
-    protected $key;
-}
-
-/**
- * @Entity
- */
-class ChildEntity extends MappedSuperclass
-{
 }
 
 /**