From d6b5d704a669065e5d18d1176a84e9a8fbe192d9 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Mon, 8 Feb 2021 01:39:31 +0100 Subject: [PATCH] [GH-8410] Fix memory leak in new toIterable and state bug. The new AbstractQuery::toIterable() had a memory leak that AbstractQuery::iterable() did not have. This leak is now fixed. After fixing the leak, one test failed where the identity map in ObjectHydrator triggered and lead to a notice. Introduced a new AbstractHydrator::cleanupAfterRowIteration() that the ObjectHydrator uses to cleanup the state. --- .../ORM/Internal/Hydration/AbstractHydrator.php | 10 ++++++++-- .../ORM/Internal/Hydration/ObjectHydrator.php | 8 ++++++++ .../Functional/Ticket/GH7496WithToIterableTest.php | 11 +++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index f84b1355411..d6f3c817d87 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -165,8 +165,6 @@ public function toIterable(Statement $stmt, ResultSetMapping $resultSetMapping, $this->prepare(); - $result = []; - while (true) { $row = $this->_stmt->fetch(FetchMode::ASSOCIATIVE); @@ -176,8 +174,12 @@ public function toIterable(Statement $stmt, ResultSetMapping $resultSetMapping, break; } + $result = []; + $this->hydrateRowData($row, $result); + $this->cleanupAfterRowIteration(); + yield end($result); } } @@ -274,6 +276,10 @@ protected function cleanup() ->removeEventListener([Events::onClear], $this); } + protected function cleanupAfterRowIteration(): void + { + } + /** * Hydrates a single row from the current statement instance. * diff --git a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php index 03ea08523ac..7bcee57b8df 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php @@ -141,6 +141,14 @@ protected function cleanup() $this->_uow->hydrationComplete(); } + protected function cleanupAfterRowIteration(): void + { + $this->identifierMap = + $this->initializedCollections = + $this->existingCollections = + $this->resultPointers = []; + } + /** * {@inheritdoc} */ diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7496WithToIterableTest.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7496WithToIterableTest.php index 1d91b41f359..2a16bc0c956 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7496WithToIterableTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7496WithToIterableTest.php @@ -41,9 +41,20 @@ public function testNonUniqueObjectHydrationDuringIteration(): void $bs = IterableTester::iterableToArray( $q->toIterable([], AbstractQuery::HYDRATE_OBJECT) ); + $this->assertCount(2, $bs); $this->assertInstanceOf(GH7496EntityB::class, $bs[0]); $this->assertInstanceOf(GH7496EntityB::class, $bs[1]); + $this->assertEquals(1, $bs[0]->id); + $this->assertEquals(1, $bs[1]->id); + + $bs = IterableTester::iterableToArray( + $q->toIterable([], AbstractQuery::HYDRATE_ARRAY) + ); + + $this->assertCount(2, $bs); + $this->assertEquals(1, $bs[0]['id']); + $this->assertEquals(1, $bs[1]['id']); } }