Skip to content

Commit

Permalink
bugfix: deallocate mysqli prepared statement
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.
  • Loading branch information
petermein committed Jan 7, 2025
1 parent 0ac3d64 commit 29fe1e0
Show file tree
Hide file tree
Showing 3 changed files with 78 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(

Check warning on line 54 in src/Driver/Mysqli/Result.php

View check run for this annotation

Codecov / codecov/patch

src/Driver/Mysqli/Result.php#L54

Added line #L54 was not covered by tests
mysqli_stmt $statement,
?Statement $statementReference = null
) {
$this->statement = $statement;
$this->statementReference = $statementReference;

Check warning on line 59 in src/Driver/Mysqli/Result.php

View check run for this annotation

Codecov / codecov/patch

src/Driver/Mysqli/Result.php#L58-L59

Added lines #L58 - L59 were not covered by tests

$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()

Check warning on line 64 in src/Driver/Mysqli/Statement.php

View check run for this annotation

Codecov / codecov/patch

src/Driver/Mysqli/Statement.php#L64

Added line #L64 was not covered by tests
{
@$this->stmt->close();

Check warning on line 66 in src/Driver/Mysqli/Statement.php

View check run for this annotation

Codecov / codecov/patch

src/Driver/Mysqli/Statement.php#L66

Added line #L66 was not covered by tests
}

/**
* @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);

Check warning on line 167 in src/Driver/Mysqli/Statement.php

View check run for this annotation

Codecov / codecov/patch

src/Driver/Mysqli/Statement.php#L167

Added line #L167 was not covered by tests
}

/**
Expand Down
57 changes: 57 additions & 0 deletions tests/Functional/Driver/Mysqli/StatementTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?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 ReflectionProperty;

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);

Check failure on line 45 in tests/Functional/Driver/Mysqli/StatementTest.php

View workflow job for this annotation

GitHub Actions / Coding Standards / Coding Standards (PHP: 8.4)

Functions must not contain multiple empty lines in a row; found 2 empty lines

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

$mysqliStatement->execute();
}
}

0 comments on commit 29fe1e0

Please sign in to comment.