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 3, 2025
1 parent 6428f1a commit ea38c64
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 2 deletions.
7 changes: 6 additions & 1 deletion src/Driver/Mysqli/Result.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,14 @@ final class Result implements ResultInterface
/**
* @internal The result can be only instantiated by its driver connection or statement.
*
* @param Statement|null $statementReference This reference is to not trigger destruction of the Statement object too early.

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

View workflow job for this annotation

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

Line exceeds 120 characters; contains 128 characters
*
* @throws Exception
*/
public function __construct(private readonly mysqli_stmt $statement)
public function __construct(

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

View check run for this annotation

Codecov / codecov/patch

src/Driver/Mysqli/Result.php#L46

Added line #L46 was not covered by tests
private readonly mysqli_stmt $statement,
private readonly ?Statement $statementReference = null,

Check failure on line 48 in src/Driver/Mysqli/Result.php

View workflow job for this annotation

GitHub Actions / Static Analysis with PHPStan (8.3)

Property Doctrine\DBAL\Driver\Mysqli\Result::$statementReference is never read, only written.
)
{

Check failure on line 50 in src/Driver/Mysqli/Result.php

View workflow job for this annotation

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

The closing parenthesis and the opening brace of a multi-line function declaration must be on the same line
$meta = $statement->result_metadata();
$this->hasColumns = $meta !== false;
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 @@ -49,6 +49,11 @@ public function __construct(private readonly mysqli_stmt $stmt)
$this->boundValues = array_fill(1, $paramCount, null);
}

public function __destruct()

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

View check run for this annotation

Codecov / codecov/patch

src/Driver/Mysqli/Statement.php#L52

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

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

View check run for this annotation

Codecov / codecov/patch

src/Driver/Mysqli/Statement.php#L54

Added line #L54 was not covered by tests
}

public function bindValue(int|string $param, mixed $value, ParameterType $type): void
{
assert(is_int($param));
Expand All @@ -72,7 +77,7 @@ public function execute(): Result
throw StatementError::upcast($e);
}

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

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

View check run for this annotation

Codecov / codecov/patch

src/Driver/Mysqli/Statement.php#L80

Added line #L80 was not covered by tests
}

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

declare(strict_types=1);

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

use Doctrine\DBAL\Driver\Mysqli\Statement;
use Doctrine\DBAL\Exception\DriverException;

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

View workflow job for this annotation

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

Type Doctrine\DBAL\Exception\DriverException is not used in this file.
use Doctrine\DBAL\Statement as WrapperStatement;
use Doctrine\DBAL\Tests\FunctionalTestCase;
use Doctrine\DBAL\Tests\TestUtil;
use Error;
use mysqli_sql_exception;

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

View workflow job for this annotation

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

Type mysqli_sql_exception is not used in this file.
use PHPUnit\Framework\Attributes\RequiresPhpExtension;
use ReflectionProperty;

#[RequiresPhpExtension('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);

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

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

unset($statement, $driverStatement);

$this->expectException(Error::class);
$this->expectExceptionMessage('mysqli_stmt object is already closed');

$mysqliStatement->execute();
}
}

0 comments on commit ea38c64

Please sign in to comment.