phql_internal_parse_phql() memory leak fix

[nope7777]

Hello!

• Type: bug fix
• Link to issue: -

In raising this pull request, I confirm the following:

• I have read and understood the Contributing Guidelines
• I have checked that another pull request for this purpose does not exist
• I wrote some tests for this PR
• I have updated the relevant CHANGELOG
• I have created a PR for the documentation about this change

In long-running processes (like queue consumers) I call Phalcon\Mvc\Model\Manager::__destruct() to clear the PHQL cache. This helps to prevent rapid memory growth, but memory usage still continues to slowly increase over time.

It appears that the issue is caused by a call to Z_TRY_ADDREF_P inside phql_internal_parse_phql:

int phql_internal_parse_phql(zval **result, char *phql, unsigned int phql_length, zval **error_msg)
{
	// ...

	cache_level = phalcon_globals_ptr->orm.cache_level;
	if (cache_level >= 0) {
		phql_key = zend_inline_hash_func(phql, phql_length + 1);
		if (phalcon_globals_ptr->orm.parser_cache != NULL) {
			if ((temp_ast = zend_hash_index_find(phalcon_globals_ptr->orm.parser_cache, phql_key)) != NULL) {
				ZVAL_ZVAL(*result, temp_ast, 1, 0);
				Z_TRY_ADDREF_P(*result);
				return SUCCESS;
			}
		}
	}

In this case, ZVAL_ZVAL already increments the reference counter, and then it is incremented once again by Z_TRY_ADDREF_P. As a result, the reference count increases incorrectly and memory is not released as expected.

Memory consumption after applying the fix:

The issue can be reproduced locally using the following example:

<?php

use Phalcon\Di\FactoryDefault;
use Phalcon\Mvc\Model;
use Phalcon\Db\Adapter\Pdo\Sqlite;

require './vendor/autoload.php';

$di = new FactoryDefault();

$dbFile = __DIR__ . '/database.sqlite';

if (!file_exists($dbFile)) {
    touch($dbFile);
}

$di->setShared('db', function () use ($dbFile) {
    return new Sqlite([
        'dbname' => $dbFile
    ]);
});

$db = $di->get('db');

$tableExists = $db->fetchOne("
SELECT name FROM sqlite_master 
WHERE type='table' AND name='users'
");

if (!$tableExists) {
    echo "Creating table...\n";

    $db->execute("
        CREATE TABLE users (
            id INTEGER PRIMARY KEY AUTOINCREMENT,
            name TEXT,
            email TEXT
        )
    ");

    $db->execute("
        INSERT INTO users (name,email) VALUES
        ('John','john@test.com'),
        ('Anna','anna@test.com')
    ");
}

class User extends Model
{
    public $id;
    public $name;
    public $email;

    public function initialize()
    {
        $this->setSource('users');
    }
}

while (true) {
    $memory = memory_get_usage(true) / 1024 / 1024;
    $memory = round($memory, 2) . ' MB';
    var_dump($memory);

    $di->get('modelsManager')->createBuilder()
        ->addFrom(User::class)
        ->getQuery()
        ->execute();

    $di->get('modelsManager')->createBuilder()
        ->addFrom(User::class)
        ->getQuery()
        ->execute();

    $di->getShared('modelsManager')->__destruct();
    gc_collect_cycles();
}

Thanks

[nope7777]

It looks like the Windows pipeline failures are caused by php/setup-php-sdk@v0.11, not by the changes in this PR. The issue appears to be fixed in v0.12 via php/setup-php-sdk#18.