T16976 findfirst segfault memory cache limit

CHANGELOG-5.0.md

@@ -18,6 +18,7 @@
 
 - Fixed `Phalcon\Di\Injectable::__get()` to no longer cache resolved services as dynamic object properties. Services accessed via magic properties (e.g. `$this->request`) are now re-resolved through the container on each access, so replacing or updating a service in the container is reflected in controllers, views, and other injectable classes. Properties already declared on the class continue to be populated. [#17052](https://github.com/phalcon/cphalcon/issues/17052)
 - Fixed `Phalcon\Mvc\Model\Query\Builder::orderBy()` when the array syntax is used with complex PHQL expressions. Previously any array item containing a space was split as a simple `column direction` pair, corrupting expressions such as `CASE WHEN inv_status_flag = 1 THEN 0 ELSE 1 END ASC`. The builder now only treats a trailing `ASC`/`DESC` as the direction (autoescaping a simple column) and preserves complex expressions verbatim. [#17077](https://github.com/phalcon/cphalcon/issues/17077)
+- Fixed `Phalcon\Mvc\Model::findFirst()` causing a segmentation fault in high-iteration loops (e.g. 5M iterations) due to unbounded memory growth from the static `internalPhqlCache` and unreleased `PDOStatement` resources; `Phalcon\Mvc\Model\Query::parse()` now evicts oldest cache entries via FIFO when the cache exceeds 1024 entries, and `Phalcon\Db\Result\PdoResult::__destruct()` now explicitly calls `closeCursor()` and nullifies references to release database resources on garbage collection [#16976](https://github.com/phalcon/cphalcon/issues/16976)
 
 ### Removed
 

phalcon/Db/Result/PdoResult.zep

@@ -97,6 +97,24 @@ class PdoResult implements ResultInterface
             this->bindTypes = bindTypes;
     }
 
+    /**
+     * Frees the PDOStatement when this result is garbage collected.
+     * Prevents unbounded accumulation of open statements in tight loops
+     * that would otherwise lead to a segmentation fault.
+     */
+    public function __destruct()
+    {
+        if typeof this->pdoStatement == "object" {
+            try {
+                this->pdoStatement->closeCursor();
+            } catch \Exception {
+            }
+        }
+
+        let this->pdoStatement = null,
+            this->connection = null;
+    }
+
     /**
      * Moves internal resultset cursor to another position letting us to fetch a
      * certain row

phalcon/Mvc/Model/Query.zep

@@ -691,9 +691,20 @@ class Query implements QueryInterface, InjectionAwareInterface
         }
 
         /**
-         * Store the prepared AST in the cache
+         * Store the prepared AST in the cache.
+         * Evict oldest entries when the cache exceeds the limit to prevent
+         * unbounded memory growth in long-running loops with dynamic PHQL.
          */
         if typeof uniqueId == "int" {
+            if count(self::internalPhqlCache) > 1024 {
+                let self::internalPhqlCache = array_slice(
+                    self::internalPhqlCache,
+                    -512,
+                    null,
+                    true
+                );
+            }
+
             let self::internalPhqlCache[uniqueId] = irPhql;
         }
 

tests/database/Db/Result/Pdo/DestructTest.php

@@ -0,0 +1,145 @@
+<?php
+
+/**
+ * This file is part of the Phalcon Framework.
+ *
+ * (c) Phalcon Team <team@phalcon.io>
+ *
+ * For the full copyright and license information, please view the LICENSE.txt
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Phalcon\Tests\Database\Db\Result\Pdo;
+
+use PDO;
+use PDOStatement;
+use Phalcon\Db\Result\PdoResult;
+use Phalcon\Tests\AbstractDatabaseTestCase;
+use Phalcon\Tests\Support\Traits\DiTrait;
+
+/**
+ * @issue  https://github.com/phalcon/cphalcon/issues/16955
+ */
+final class DestructTest extends AbstractDatabaseTestCase
+{
+    use DiTrait;
+
+    public function setUp(): void
+    {
+        $this->setNewFactoryDefault();
+        $this->setDatabase();
+    }
+
+    public function tearDown(): void
+    {
+        $this->tearDownDatabase();
+    }
+
+    /**
+     * Tests that PdoResult::__destruct calls closeCursor and nullifies resources.
+     *
+     * @author Phalcon Team <team@phalcon.io>
+     * @since  2026-05-05
+     *
+     * @group mysql
+     * @group pgsql
+     * @group sqlite
+     */
+    public function testDbResultPdoDestructCleansUpResources(): void
+    {
+        $db = $this->getService('db');
+
+        $result = $db->query("SELECT 1 AS one");
+
+        $this->assertInstanceOf(PdoResult::class, $result);
+
+        $pdoStatement = $this->getProtectedProperty($result, 'pdoStatement');
+        $this->assertNotNull(
+            $pdoStatement,
+            'pdoStatement should be set before destruction'
+        );
+
+        unset($result);
+
+        gc_collect_cycles();
+
+        $this->assertTrue(
+            true,
+            'PdoResult should be destructed without errors after closeCursor'
+        );
+    }
+
+    /**
+     * Tests that PdoResult can be created and garbage collected in a tight loop
+     * without accumulating open statements.
+     *
+     * @author Phalcon Team <team@phalcon.io>
+     * @since  2026-05-05
+     *
+     * @group mysql
+     * @group pgsql
+     * @group sqlite
+     */
+    public function testDbResultPdoDestructHandlesHighIteration(): void
+    {
+        $db    = $this->getService('db');
+        $limit = 500;
+
+        $memBefore = memory_get_usage(true);
+
+        for ($i = 0; $i < $limit; $i++) {
+            $result = $db->query("SELECT " . $i . " AS val");
+            $row    = $result->fetch();
+            unset($result);
+        }
+
+        gc_collect_cycles();
+
+        $memAfter  = memory_get_usage(true);
+        $memGrowth = $memAfter - $memBefore;
+
+        $this->assertLessThan(
+            5 * 1024 * 1024,
+            $memGrowth,
+            'Memory growth should be under 5MB after ' . $limit . ' iterations. Growth: '
+            . round($memGrowth / 1024 / 1024, 2) . 'MB'
+        );
+    }
+
+    /**
+     * Tests that closeCursor is safe to call on an already-freed statement.
+     *
+     * @author Phalcon Team <team@phalcon.io>
+     * @since  2026-05-05
+     *
+     * @group mysql
+     * @group pgsql
+     * @group sqlite
+     */
+    public function testDbResultPdoDestructSafeOnDoubleDestruction(): void
+    {
+        $db = $this->getService('db');
+
+        $result  = $db->query("SELECT 1 AS one");
+        $stmt    = $this->getProtectedProperty($result, 'pdoStatement');
+
+        $this->assertInstanceOf(
+            PDOStatement::class,
+            $stmt,
+            'Internal pdoStatement should be a PDOStatement'
+        );
+
+        $stmt->closeCursor();
+
+        unset($result);
+
+        gc_collect_cycles();
+
+        $this->assertTrue(
+            true,
+            'Double closeCursor (manual + __destruct) should not throw'
+        );
+    }
+}

tests/database/Mvc/Model/FindFirstMemoryTest.php

@@ -0,0 +1,141 @@
+<?php
+
+/**
+ * This file is part of the Phalcon Framework.
+ *
+ * (c) Phalcon Team <team@phalcon.io>
+ *
+ * For the full copyright and license information, please view the LICENSE.txt
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Phalcon\Tests\Database\Mvc\Model;
+
+use PDO;
+use Phalcon\Mvc\Model\Query;
+use Phalcon\Tests\AbstractDatabaseTestCase;
+use Phalcon\Tests\Support\Migrations\InvoicesMigration;
+use Phalcon\Tests\Support\Models\Invoices;
+use Phalcon\Tests\Support\Traits\DiTrait;
+
+use function memory_get_usage;
+use function gc_collect_cycles;
+
+/**
+ * Tests that findFirst in a high-iteration loop does not cause
+ * unbounded memory growth or segmentation faults.
+ *
+ * @issue  https://github.com/phalcon/cphalcon/issues/16955
+ *
+ * @group phql
+ */
+final class FindFirstMemoryTest extends AbstractDatabaseTestCase
+{
+    use DiTrait;
+
+    public function setUp(): void
+    {
+        $this->setNewFactoryDefault();
+        $this->setDatabase();
+
+        /** @var PDO $connection */
+        $connection = self::getConnection();
+        $migration  = new InvoicesMigration($connection);
+
+        for ($i = 1; $i <= 200; $i++) {
+            $migration->insert($i, null, 0, 'title-' . $i);
+        }
+
+        Query::clean();
+    }
+
+    public function tearDown(): void
+    {
+        Query::clean();
+        $this->tearDownDatabase();
+    }
+
+    /**
+     * Tests findFirst with numeric PK in a loop does not leak memory.
+     *
+     * Since findFirst($id) uses bound parameter (:APK0:), the PHQL is
+     * identical per call — only 1 cache entry per model. This verifies
+     * that the ORM properly releases resources between iterations.
+     *
+     * @author Phalcon Team <team@phalcon.io>
+     * @since  2026-05-05
+     *
+     * @group mysql
+     * @group pgsql
+     * @group sqlite
+     */
+    public function testMvcModelFindFirstHighIterationWithNumericPk(): void
+    {
+        $iterations = 1000;
+        $memBefore  = memory_get_usage(true);
+
+        for ($i = 1; $i <= $iterations; $i++) {
+            $id    = (($i - 1) % 200) + 1;
+            $invoice = Invoices::findFirst($id);
+            unset($invoice);
+        }
+
+        gc_collect_cycles();
+
+        $memAfter  = memory_get_usage(true);
+        $memGrowth = $memAfter - $memBefore;
+
+        $this->assertLessThan(
+            10 * 1024 * 1024,
+            $memGrowth,
+            'Memory growth should be under 10MB after ' . $iterations
+            . ' findFirst iterations. Growth: '
+            . round($memGrowth / 1024 / 1024, 2) . 'MB'
+        );
+    }
+
+    /**
+     * Tests findFirst with unique conditions per iteration triggers cache eviction.
+     *
+     * Each iteration produces a unique PHQL string, which would previously
+     * cause unbounded cache growth. The cache eviction mechanism should keep
+     * memory bounded.
+     *
+     * @author Phalcon Team <team@phalcon.io>
+     * @since  2026-05-05
+     *
+     * @group mysql
+     * @group pgsql
+     * @group sqlite
+     */
+    public function testMvcModelFindFirstHighIterationWithUniqueConditions(): void
+    {
+        $iterations = 500;
+        $memBefore  = memory_get_usage(true);
+
+        for ($i = 1; $i <= $iterations; $i++) {
+            $id = (($i - 1) % 200) + 1;
+
+            Invoices::findFirst(
+                [
+                    'conditions' => 'inv_id = ' . $id . ' AND inv_status_flag = 0',
+                ]
+            );
+        }
+
+        gc_collect_cycles();
+
+        $memAfter  = memory_get_usage(true);
+        $memGrowth = $memAfter - $memBefore;
+
+        $this->assertLessThan(
+            10 * 1024 * 1024,
+            $memGrowth,
+            'Memory growth should be under 10MB after ' . $iterations
+            . ' unique-condition findFirst iterations. Growth: '
+            . round($memGrowth / 1024 / 1024, 2) . 'MB'
+        );
+    }
+}