Skip to content

ext/pdo: Fix memory leak if GC needs to free PDO Statement #17539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions ext/pdo/pdo_stmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -2079,8 +2079,11 @@ static zend_function *dbstmt_method_get(zend_object **object_pp, zend_string *me
static HashTable *dbstmt_get_gc(zend_object *object, zval **gc_data, int *gc_count)
{
pdo_stmt_t *stmt = php_pdo_stmt_fetch_object(object);
*gc_data = &stmt->fetch.into;
*gc_count = 1;

zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create();
zend_get_gc_buffer_add_zval(gc_buffer, &stmt->database_object_handle);
zend_get_gc_buffer_add_zval(gc_buffer, &stmt->fetch.into);
zend_get_gc_buffer_use(gc_buffer, gc_data, gc_count);

/**
* If there are no dynamic properties and the default property is 1 (that is, there is only one property
Expand Down
132 changes: 132 additions & 0 deletions ext/pdo/tests/pdo_stmt_cyclic_references.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
--TEST--
PDO Common: Cyclic PDOStatement child class
--EXTENSIONS--
pdo
--SKIPIF--
<?php
$dir = getenv('REDIR_TEST_DIR');
if (false == $dir) die('skip no driver');
require_once $dir . 'pdo_test.inc';
PDOTest::skip();
?>
--FILE--
<?php
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';

class Ref {
public CyclicStatement $stmt;
}

class CyclicStatement extends PDOStatement {
protected function __construct(public Ref $ref) {}
}

class TestRow {
public $id;
public $val;
public $val2;

public function __construct(public string $arg) {}
}

$db = PDOTest::factory();
$db->exec('CREATE TABLE test(id INT NOT NULL PRIMARY KEY, val VARCHAR(10), val2 VARCHAR(10))');
$db->exec("INSERT INTO test VALUES(1, 'A', 'AA')");
$db->exec("INSERT INTO test VALUES(2, 'B', 'BB')");
$db->exec("INSERT INTO test VALUES(3, 'C', 'CC')");

$db->setAttribute(PDO::ATTR_STATEMENT_CLASS, ['CyclicStatement', [new Ref]]);

echo "Column fetch:\n";
$stmt = $db->query('SELECT id, val2, val FROM test');
$stmt->ref->stmt = $stmt;
$stmt->setFetchMode(PDO::FETCH_COLUMN, 2);
foreach($stmt as $obj) {
var_dump($obj);
}

echo "Class fetch:\n";
$stmt = $db->query('SELECT id, val2, val FROM test');
$stmt->ref->stmt = $stmt;
$stmt->setFetchMode(PDO::FETCH_CLASS, 'TestRow', ['Hello world']);
foreach($stmt as $obj) {
var_dump($obj);
}

echo "Fetch into:\n";
$stmt = $db->query('SELECT id, val2, val FROM test');
$stmt->ref->stmt = $stmt;
$stmt->setFetchMode(PDO::FETCH_INTO, new TestRow('I am being fetch into'));
foreach($stmt as $obj) {
var_dump($obj);
}

?>
--EXPECTF--
Column fetch:
string(1) "A"
string(1) "B"
string(1) "C"
Class fetch:
object(TestRow)#%d (4) {
["id"]=>
string(1) "1"
["val"]=>
string(1) "A"
["val2"]=>
string(2) "AA"
["arg"]=>
string(11) "Hello world"
}
object(TestRow)#%d (4) {
["id"]=>
string(1) "2"
["val"]=>
string(1) "B"
["val2"]=>
string(2) "BB"
["arg"]=>
string(11) "Hello world"
}
object(TestRow)#%d (4) {
["id"]=>
string(1) "3"
["val"]=>
string(1) "C"
["val2"]=>
string(2) "CC"
["arg"]=>
string(11) "Hello world"
}
Fetch into:
object(TestRow)#4 (4) {
["id"]=>
string(1) "1"
["val"]=>
string(1) "A"
["val2"]=>
string(2) "AA"
["arg"]=>
string(21) "I am being fetch into"
}
object(TestRow)#4 (4) {
["id"]=>
string(1) "2"
["val"]=>
string(1) "B"
["val2"]=>
string(2) "BB"
["arg"]=>
string(21) "I am being fetch into"
}
object(TestRow)#4 (4) {
["id"]=>
string(1) "3"
["val"]=>
string(1) "C"
["val2"]=>
string(2) "CC"
["arg"]=>
string(21) "I am being fetch into"
}
11 changes: 9 additions & 2 deletions ext/pdo_firebird/firebird_statement.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,15 @@ static int firebird_stmt_dtor(pdo_stmt_t *stmt) /* {{{ */
pdo_firebird_stmt *S = (pdo_firebird_stmt*)stmt->driver_data;
int result = 1;

/* release the statement */
if (isc_dsql_free_statement(S->H->isc_status, &S->stmt, DSQL_drop)) {
/* TODO: for master, move this check to a separate function shared between pdo drivers.
* pdo_pgsql and pdo_mysql do this exact same thing */
bool server_obj_usable = !Z_ISUNDEF(stmt->database_object_handle)
&& IS_OBJ_VALID(EG(objects_store).object_buckets[Z_OBJ_HANDLE(stmt->database_object_handle)])
&& !(OBJ_FLAGS(Z_OBJ(stmt->database_object_handle)) & IS_OBJ_FREE_CALLED);

/* release the statement.
* Note: if the server object is already gone then the statement was closed already as well. */
if (server_obj_usable && isc_dsql_free_statement(S->H->isc_status, &S->stmt, DSQL_drop)) {
RECORD_ERROR(stmt);
result = 0;
}
Expand Down
Loading