From 521656d0fdd5ff67e32295f01eb1ed99bd5f28ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= <jerome@tamarelle.net>
Date: Thu, 19 Dec 2024 00:19:07 +0100
Subject: [PATCH 1/4] Replace wrapTransversable generators with
 IteratorIterator to prevent memory leaks

---
 .../ODM/MongoDB/Iterator/CachingIterator.php  | 45 +++++---------
 .../MongoDB/Iterator/HydratingIterator.php    | 24 ++------
 .../MongoDB/Iterator/UnrewindableIterator.php | 61 ++++++++-----------
 .../Iterator/UnrewindableIteratorTest.php     |  9 +++
 4 files changed, 55 insertions(+), 84 deletions(-)

diff --git a/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php b/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php
index d726712013..223f541740 100644
--- a/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php
+++ b/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php
@@ -5,7 +5,8 @@
 namespace Doctrine\ODM\MongoDB\Iterator;
 
 use Countable;
-use Generator;
+use Iterator;
+use IteratorIterator;
 use ReturnTypeWillChange;
 use RuntimeException;
 use Traversable;
@@ -33,13 +34,11 @@ final class CachingIterator implements Countable, Iterator
     /** @var array<mixed, TValue> */
     private array $items = [];
 
-    /** @var Generator<mixed, TValue>|null */
-    private ?Generator $iterator;
+    /** @var Iterator<mixed, TValue>|null */
+    private ?Iterator $iterator;
 
     private bool $iteratorAdvanced = false;
 
-    private bool $iteratorExhausted = false;
-
     /**
      * Initialize the iterator and stores the first item in the cache. This
      * effectively rewinds the Traversable and the wrapping Generator, which
@@ -51,7 +50,8 @@ final class CachingIterator implements Countable, Iterator
      */
     public function __construct(Traversable $iterator)
     {
-        $this->iterator = $this->wrapTraversable($iterator);
+        $this->iterator = new IteratorIterator($iterator);
+        $this->iterator->rewind();
         $this->storeCurrentItem();
     }
 
@@ -94,9 +94,14 @@ public function key()
     /** @see http://php.net/iterator.next */
     public function next(): void
     {
-        if (! $this->iteratorExhausted) {
-            $this->getIterator()->next();
+        if ($this->iterator !== null) {
+            $this->iterator->next();
             $this->storeCurrentItem();
+            $this->iteratorAdvanced = true;
+
+            if (! $this->iterator->valid()) {
+                $this->iterator = null;
+            }
         }
 
         next($this->items);
@@ -126,15 +131,13 @@ public function valid(): bool
      */
     private function exhaustIterator(): void
     {
-        while (! $this->iteratorExhausted) {
+        while ($this->iterator !== null) {
             $this->next();
         }
-
-        $this->iterator = null;
     }
 
-    /** @return Generator<mixed, TValue> */
-    private function getIterator(): Generator
+    /** @return Iterator<mixed, TValue> */
+    private function getIterator(): Iterator
     {
         if ($this->iterator === null) {
             throw new RuntimeException('Iterator has already been destroyed');
@@ -156,20 +159,4 @@ private function storeCurrentItem(): void
 
         $this->items[$key] = $this->getIterator()->current();
     }
-
-    /**
-     * @param Traversable<mixed, TValue> $traversable
-     *
-     * @return Generator<mixed, TValue>
-     */
-    private function wrapTraversable(Traversable $traversable): Generator
-    {
-        foreach ($traversable as $key => $value) {
-            yield $key => $value;
-
-            $this->iteratorAdvanced = true;
-        }
-
-        $this->iteratorExhausted = true;
-    }
 }
diff --git a/lib/Doctrine/ODM/MongoDB/Iterator/HydratingIterator.php b/lib/Doctrine/ODM/MongoDB/Iterator/HydratingIterator.php
index 94f5f219f6..c1450cc281 100644
--- a/lib/Doctrine/ODM/MongoDB/Iterator/HydratingIterator.php
+++ b/lib/Doctrine/ODM/MongoDB/Iterator/HydratingIterator.php
@@ -6,8 +6,8 @@
 
 use Doctrine\ODM\MongoDB\Mapping\ClassMetadata;
 use Doctrine\ODM\MongoDB\UnitOfWork;
-use Generator;
 use Iterator;
+use IteratorIterator;
 use ReturnTypeWillChange;
 use RuntimeException;
 use Traversable;
@@ -24,8 +24,8 @@
  */
 final class HydratingIterator implements Iterator
 {
-    /** @var Generator<mixed, array<string, mixed>>|null */
-    private ?Generator $iterator;
+    /** @var Iterator<mixed, array<string, mixed>>|null */
+    private ?Iterator $iterator;
 
     /**
      * @param Traversable<mixed, array<string, mixed>> $traversable
@@ -34,7 +34,7 @@ final class HydratingIterator implements Iterator
      */
     public function __construct(Traversable $traversable, private UnitOfWork $unitOfWork, private ClassMetadata $class, private array $unitOfWorkHints = [])
     {
-        $this->iterator = $this->wrapTraversable($traversable);
+        $this->iterator = new IteratorIterator($traversable);
     }
 
     public function __destruct()
@@ -74,8 +74,8 @@ public function valid(): bool
         return $this->key() !== null;
     }
 
-    /** @return Generator<mixed, array<string, mixed>> */
-    private function getIterator(): Generator
+    /** @return Iterator<mixed, array<string, mixed>> */
+    private function getIterator(): Iterator
     {
         if ($this->iterator === null) {
             throw new RuntimeException('Iterator has already been destroyed');
@@ -93,16 +93,4 @@ private function hydrate(?array $document): ?object
     {
         return $document !== null ? $this->unitOfWork->getOrCreateDocument($this->class->name, $document, $this->unitOfWorkHints) : null;
     }
-
-    /**
-     * @param Traversable<mixed, array<string, mixed>> $traversable
-     *
-     * @return Generator<mixed, array<string, mixed>>
-     */
-    private function wrapTraversable(Traversable $traversable): Generator
-    {
-        foreach ($traversable as $key => $value) {
-            yield $key => $value;
-        }
-    }
 }
diff --git a/lib/Doctrine/ODM/MongoDB/Iterator/UnrewindableIterator.php b/lib/Doctrine/ODM/MongoDB/Iterator/UnrewindableIterator.php
index f13baed355..36a9c3ba7e 100644
--- a/lib/Doctrine/ODM/MongoDB/Iterator/UnrewindableIterator.php
+++ b/lib/Doctrine/ODM/MongoDB/Iterator/UnrewindableIterator.php
@@ -4,7 +4,8 @@
 
 namespace Doctrine\ODM\MongoDB\Iterator;
 
-use Generator;
+use Iterator;
+use IteratorIterator;
 use LogicException;
 use ReturnTypeWillChange;
 use RuntimeException;
@@ -23,39 +24,34 @@
  */
 final class UnrewindableIterator implements Iterator
 {
-    /** @var Generator<mixed, TValue>|null */
-    private ?Generator $iterator;
+    /** @var Iterator<mixed, TValue>|null */
+    private ?Iterator $iterator;
 
     private bool $iteratorAdvanced = false;
 
     /**
-     * Initialize the iterator. This effectively rewinds the Traversable and
-     * the wrapping Generator, which will execute up to its first yield statement.
-     * Additionally, this mimics behavior of the SPL iterators and allows users
-     * to omit an explicit call to rewind() before using the other methods.
+     * Initialize the iterator. This effectively rewinds the Traversable.
+     * This mimics behavior of the SPL iterators and allows users to omit an
+     * explicit call to rewind() before using the other methods.
      *
      * @param Traversable<mixed, TValue> $iterator
      */
     public function __construct(Traversable $iterator)
     {
-        $this->iterator = $this->wrapTraversable($iterator);
-        $this->iterator->key();
+        $this->iterator = new IteratorIterator($iterator);
+        $this->iterator->rewind();
     }
 
     public function toArray(): array
     {
         $this->preventRewinding(__METHOD__);
 
-        $toArray = function () {
-            if (! $this->valid()) {
-                return;
-            }
-
-            yield $this->key() => $this->current();
-            yield from $this->getIterator();
-        };
-
-        return iterator_to_array($toArray());
+        try {
+            return iterator_to_array($this->getIterator());
+        } finally {
+            $this->iteratorAdvanced = true;
+            $this->iterator         = null;
+        }
     }
 
     /** @return TValue|null */
@@ -84,6 +80,13 @@ public function next(): void
         }
 
         $this->iterator->next();
+        $this->iteratorAdvanced = true;
+
+        if ($this->iterator->valid()) {
+            return;
+        }
+
+        $this->iterator = null;
     }
 
     /** @see http://php.net/iterator.rewind */
@@ -108,8 +111,8 @@ private function preventRewinding(string $method): void
         }
     }
 
-    /** @return Generator<mixed, TValue> */
-    private function getIterator(): Generator
+    /** @return Iterator<mixed, TValue> */
+    private function getIterator(): Iterator
     {
         if ($this->iterator === null) {
             throw new RuntimeException('Iterator has already been destroyed');
@@ -117,20 +120,4 @@ private function getIterator(): Generator
 
         return $this->iterator;
     }
-
-    /**
-     * @param Traversable<mixed, TValue> $traversable
-     *
-     * @return Generator<mixed, TValue>
-     */
-    private function wrapTraversable(Traversable $traversable): Generator
-    {
-        foreach ($traversable as $key => $value) {
-            yield $key => $value;
-
-            $this->iteratorAdvanced = true;
-        }
-
-        $this->iterator = null;
-    }
 }
diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/UnrewindableIteratorTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/UnrewindableIteratorTest.php
index 304724c2ac..ef0e05e83b 100644
--- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/UnrewindableIteratorTest.php
+++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/UnrewindableIteratorTest.php
@@ -100,6 +100,15 @@ public function testRewindAfterPartialIteration(): void
         iterator_to_array($iterator);
     }
 
+    public function testRewingAfterToArray(): void
+    {
+        $iterator = new UnrewindableIterator($this->getTraversable([1, 2, 3]));
+
+        $iterator->toArray();
+        $this->expectException(LogicException::class);
+        $iterator->rewind();
+    }
+
     public function testToArray(): void
     {
         $iterator = new UnrewindableIterator($this->getTraversable([1, 2, 3]));

From 153ea2aa3b2d377694d11fd67e4bf3fc0431f530 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= <jerome@tamarelle.net>
Date: Thu, 19 Dec 2024 01:07:27 +0100
Subject: [PATCH 2/4] Fix implemented interface

---
 lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php  | 10 +++++-----
 .../ODM/MongoDB/Iterator/UnrewindableIterator.php      | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php b/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php
index 223f541740..4caa36843d 100644
--- a/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php
+++ b/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php
@@ -5,7 +5,7 @@
 namespace Doctrine\ODM\MongoDB\Iterator;
 
 use Countable;
-use Iterator;
+use Iterator as SPLIterator;
 use IteratorIterator;
 use ReturnTypeWillChange;
 use RuntimeException;
@@ -34,8 +34,8 @@ final class CachingIterator implements Countable, Iterator
     /** @var array<mixed, TValue> */
     private array $items = [];
 
-    /** @var Iterator<mixed, TValue>|null */
-    private ?Iterator $iterator;
+    /** @var SPLIterator<mixed, TValue>|null */
+    private ?SPLIterator $iterator;
 
     private bool $iteratorAdvanced = false;
 
@@ -136,8 +136,8 @@ private function exhaustIterator(): void
         }
     }
 
-    /** @return Iterator<mixed, TValue> */
-    private function getIterator(): Iterator
+    /** @return SPLIterator<mixed, TValue> */
+    private function getIterator(): SPLIterator
     {
         if ($this->iterator === null) {
             throw new RuntimeException('Iterator has already been destroyed');
diff --git a/lib/Doctrine/ODM/MongoDB/Iterator/UnrewindableIterator.php b/lib/Doctrine/ODM/MongoDB/Iterator/UnrewindableIterator.php
index 36a9c3ba7e..c0352c050e 100644
--- a/lib/Doctrine/ODM/MongoDB/Iterator/UnrewindableIterator.php
+++ b/lib/Doctrine/ODM/MongoDB/Iterator/UnrewindableIterator.php
@@ -4,7 +4,7 @@
 
 namespace Doctrine\ODM\MongoDB\Iterator;
 
-use Iterator;
+use Iterator as SPLIterator;
 use IteratorIterator;
 use LogicException;
 use ReturnTypeWillChange;
@@ -24,8 +24,8 @@
  */
 final class UnrewindableIterator implements Iterator
 {
-    /** @var Iterator<mixed, TValue>|null */
-    private ?Iterator $iterator;
+    /** @var SPLIterator<mixed, TValue>|null */
+    private ?SPLIterator $iterator;
 
     private bool $iteratorAdvanced = false;
 
@@ -111,8 +111,8 @@ private function preventRewinding(string $method): void
         }
     }
 
-    /** @return Iterator<mixed, TValue> */
-    private function getIterator(): Iterator
+    /** @return SPLIterator<mixed, TValue> */
+    private function getIterator(): SPLIterator
     {
         if ($this->iterator === null) {
             throw new RuntimeException('Iterator has already been destroyed');

From db1f3684e52207fa54833bf733cf12019f04f083 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= <jerome@tamarelle.net>
Date: Thu, 19 Dec 2024 02:45:55 +0100
Subject: [PATCH 3/4] Fix for dead cursor

---
 .../ODM/MongoDB/Iterator/CachingIterator.php        |  6 +++++-
 .../ODM/MongoDB/Iterator/HydratingIterator.php      |  1 +
 .../Functional/Iterator/CachingIteratorTest.php     | 13 +++++++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php b/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php
index 4caa36843d..5d10d490c0 100644
--- a/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php
+++ b/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php
@@ -52,7 +52,11 @@ public function __construct(Traversable $iterator)
     {
         $this->iterator = new IteratorIterator($iterator);
         $this->iterator->rewind();
-        $this->storeCurrentItem();
+        if ($this->iterator->valid()) {
+            $this->storeCurrentItem();
+        } else {
+            $this->iterator = null;
+        }
     }
 
     /** @see https://php.net/countable.count */
diff --git a/lib/Doctrine/ODM/MongoDB/Iterator/HydratingIterator.php b/lib/Doctrine/ODM/MongoDB/Iterator/HydratingIterator.php
index c1450cc281..1e1433cdb1 100644
--- a/lib/Doctrine/ODM/MongoDB/Iterator/HydratingIterator.php
+++ b/lib/Doctrine/ODM/MongoDB/Iterator/HydratingIterator.php
@@ -35,6 +35,7 @@ final class HydratingIterator implements Iterator
     public function __construct(Traversable $traversable, private UnitOfWork $unitOfWork, private ClassMetadata $class, private array $unitOfWorkHints = [])
     {
         $this->iterator = new IteratorIterator($traversable);
+        $this->iterator->rewind();
     }
 
     public function __destruct()
diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/CachingIteratorTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/CachingIteratorTest.php
index 52b90c1dec..dc8373f115 100644
--- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/CachingIteratorTest.php
+++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/CachingIteratorTest.php
@@ -59,6 +59,19 @@ public function testIterationWithEmptySet(): void
         self::assertFalse($iterator->valid());
     }
 
+    public function testIterationWithInvalidIterator(): void
+    {
+        $mock = $this->createMock(Iterator::class);
+        // The method next() should not be called on a dead cursor.
+        $mock->expects(self::never())->method('next');
+        // The method valid() return false on a dead cursor.
+        $mock->expects(self::once())->method('valid')->willReturn(false);
+
+        $iterator = new CachingIterator($mock);
+
+        $this->assertEquals([], $iterator->toArray());
+    }
+
     public function testPartialIterationDoesNotExhaust(): void
     {
         $traversable = $this->getTraversableThatThrows([1, 2, new Exception()]);

From 5a09b65b8ef8045ac03d6a0261b10ac39922f0f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= <jerome@tamarelle.net>
Date: Thu, 19 Dec 2024 09:17:11 +0100
Subject: [PATCH 4/4] Factorize into storeCurrentItem

---
 .../ODM/MongoDB/Iterator/CachingIterator.php   | 18 +++++-------------
 .../Iterator/UnrewindableIteratorTest.php      |  2 +-
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php b/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php
index 5d10d490c0..8d80284fa2 100644
--- a/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php
+++ b/lib/Doctrine/ODM/MongoDB/Iterator/CachingIterator.php
@@ -52,11 +52,7 @@ public function __construct(Traversable $iterator)
     {
         $this->iterator = new IteratorIterator($iterator);
         $this->iterator->rewind();
-        if ($this->iterator->valid()) {
-            $this->storeCurrentItem();
-        } else {
-            $this->iterator = null;
-        }
+        $this->storeCurrentItem();
     }
 
     /** @see https://php.net/countable.count */
@@ -102,10 +98,6 @@ public function next(): void
             $this->iterator->next();
             $this->storeCurrentItem();
             $this->iteratorAdvanced = true;
-
-            if (! $this->iterator->valid()) {
-                $this->iterator = null;
-            }
         }
 
         next($this->items);
@@ -155,12 +147,12 @@ private function getIterator(): SPLIterator
      */
     private function storeCurrentItem(): void
     {
-        $key = $this->getIterator()->key();
+        $key = $this->iterator->key();
 
         if ($key === null) {
-            return;
+            $this->iterator = null;
+        } else {
+            $this->items[$key] = $this->getIterator()->current();
         }
-
-        $this->items[$key] = $this->getIterator()->current();
     }
 }
diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/UnrewindableIteratorTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/UnrewindableIteratorTest.php
index ef0e05e83b..1284204256 100644
--- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/UnrewindableIteratorTest.php
+++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Iterator/UnrewindableIteratorTest.php
@@ -100,7 +100,7 @@ public function testRewindAfterPartialIteration(): void
         iterator_to_array($iterator);
     }
 
-    public function testRewingAfterToArray(): void
+    public function testRewindAfterToArray(): void
     {
         $iterator = new UnrewindableIterator($this->getTraversable([1, 2, 3]));