Skip to content

Commit

Permalink
bugfix: deallocate mysqli prepared statement (#6681)
Browse files Browse the repository at this point in the history
Long running processes might hit the `max_prepared_stmt_count` due not
deallocating the statement correctly.

<!-- Fill in the relevant information below to help triage your pull
request. -->

|      Q       |   A
|------------- | -----------
| Type         | bug
| Fixed issues | -

#### Summary

For postgres there was an implementation to deallocate prepared
statements to prevent hitting a any limits. This was not the case for
the `mysqli` drivers. Which can suffer from the same problem.

In this PR a `mysqli_stmt_close` is added to properly cleanup any open
prepared statement.

A similar approach was implemented for PostgreSQL in this PR
#5893

---------

Co-authored-by: Alexander M. Turek <[email protected]>
  • Loading branch information
petermein and derrabus authored Jan 16, 2025
1 parent 9f28dee commit ec16c82
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 4 deletions.
18 changes: 15 additions & 3 deletions src/Driver/Mysqli/Result.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ final class Result implements ResultInterface
{
private mysqli_stmt $statement;

/**
* Maintains a reference to the Statement that generated this result. This ensures that the lifetime of the
* Statement is managed in conjunction with its associated results, so they are destroyed together
* at the appropriate time {@see Statement::__destruct()}.
*
* @phpstan-ignore property.onlyWritten
*/
private ?Statement $statementReference = null;

/**
* Whether the statement result has columns. The property should be used only after the result metadata
* has been fetched ({@see $metadataFetched}). Otherwise, the property value is undetermined.
Expand All @@ -42,9 +51,12 @@ final class Result implements ResultInterface
*
* @throws Exception
*/
public function __construct(mysqli_stmt $statement)
{
$this->statement = $statement;
public function __construct(
mysqli_stmt $statement,
?Statement $statementReference = null
) {
$this->statement = $statement;
$this->statementReference = $statementReference;

$meta = $statement->result_metadata();

Expand Down
7 changes: 6 additions & 1 deletion src/Driver/Mysqli/Statement.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ public function __construct(mysqli_stmt $stmt)
$this->boundValues = array_fill(1, $paramCount, null);
}

public function __destruct()
{
@$this->stmt->close();
}

/**
* @deprecated Use {@see bindValue()} instead.
*
Expand Down Expand Up @@ -159,7 +164,7 @@ public function execute($params = null): ResultInterface
throw StatementError::new($this->stmt);
}

return new Result($this->stmt);
return new Result($this->stmt, $this);
}

/**
Expand Down
68 changes: 68 additions & 0 deletions tests/Functional/Driver/Mysqli/StatementTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Tests\Functional\Driver\Mysqli;

use Doctrine\DBAL\Driver\Mysqli\Statement;
use Doctrine\DBAL\Statement as WrapperStatement;
use Doctrine\DBAL\Tests\FunctionalTestCase;
use Doctrine\DBAL\Tests\TestUtil;
use Error;
use ErrorException;
use ReflectionProperty;

use function restore_error_handler;
use function set_error_handler;

use const PHP_VERSION_ID;

/** @requires extension mysqli */
class StatementTest extends FunctionalTestCase
{
protected function setUp(): void
{
parent::setUp();

if (TestUtil::isDriverOneOf('mysqli')) {
return;
}

self::markTestSkipped('This test requires the mysqli driver.');
}

public function testStatementsAreDeallocatedProperly(): void
{
$statement = $this->connection->prepare('SELECT 1');

$property = new ReflectionProperty(WrapperStatement::class, 'stmt');
$property->setAccessible(true);

$driverStatement = $property->getValue($statement);

$mysqliProperty = new ReflectionProperty(Statement::class, 'stmt');
$mysqliProperty->setAccessible(true);

$mysqliStatement = $mysqliProperty->getValue($driverStatement);

unset($statement, $driverStatement);

if (PHP_VERSION_ID < 80000) {
$this->expectException(ErrorException::class);
$this->expectExceptionMessage('mysqli_stmt::execute(): Couldn\'t fetch mysqli_stmt');
} else {
$this->expectException(Error::class);
$this->expectExceptionMessage('mysqli_stmt object is already closed');
}

set_error_handler(static function (int $errorNumber, string $error, string $file, int $line): void {
throw new ErrorException($error, 0, $errorNumber, $file, $line);
});

try {
$mysqliStatement->execute();
} finally {
restore_error_handler();
}
}
}

0 comments on commit ec16c82

Please sign in to comment.