From 296ad2bcdd5ac36b5406d9a44040cff08465bb6e Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 24 Apr 2026 23:01:56 +0100 Subject: [PATCH 1/3] Fix GH-21869: pdo_pgsql DEALLOCATE in destructor can poison the enclosing transaction. On libpq < 17 the raw DEALLOCATE can fail (e.g. Aurora DSQL rejects it), which aborts the user's transaction and turns the next COMMIT into a silent ROLLBACK. Wrap it in a SAVEPOINT so a failure is contained, and skip it when the transaction is already aborted or the connection is gone. --- ext/pdo_pgsql/pgsql_statement.c | 40 ++++++++++++++++++++++++--- ext/pdo_pgsql/tests/gh21869.phpt | 46 ++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 ext/pdo_pgsql/tests/gh21869.phpt diff --git a/ext/pdo_pgsql/pgsql_statement.c b/ext/pdo_pgsql/pgsql_statement.c index 8f3dd5237b5a..e1581602196d 100644 --- a/ext/pdo_pgsql/pgsql_statement.c +++ b/ext/pdo_pgsql/pgsql_statement.c @@ -79,9 +79,43 @@ static int pgsql_stmt_dtor(pdo_stmt_t *stmt) // TODO (??) libpq does not support close statement protocol < postgres 17 // check if we can circumvent this. char *q = NULL; - spprintf(&q, 0, "DEALLOCATE %s", S->stmt_name); - res = PQexec(H->server, q); - efree(q); + PGTransactionStatusType tstatus = PQtransactionStatus(H->server); + + switch (tstatus) { + case PQTRANS_ACTIVE: + case PQTRANS_INERROR: + case PQTRANS_UNKNOWN: + res = NULL; + break; + case PQTRANS_INTRANS: + spprintf(&q, 0, + "SAVEPOINT pdo_pgsql_deallocate_%s;" + "DEALLOCATE %s;" + "RELEASE SAVEPOINT pdo_pgsql_deallocate_%s", + S->stmt_name, + S->stmt_name, + S->stmt_name); + res = PQexec(H->server, q); + efree(q); + if (res && PQresultStatus(res) != PGRES_COMMAND_OK) { + spprintf(&q, 0, + "ROLLBACK TO SAVEPOINT pdo_pgsql_deallocate_%s;" + "RELEASE SAVEPOINT pdo_pgsql_deallocate_%s", + S->stmt_name, + S->stmt_name); + PGresult *rollres; + PQclear(res); + rollres = PQexec(H->server, q); + res = rollres; + efree(q); + } + break; + default: + spprintf(&q, 0, "DEALLOCATE %s", S->stmt_name); + res = PQexec(H->server, q); + efree(q); + break; + } #else res = PQclosePrepared(H->server, S->stmt_name); #endif diff --git a/ext/pdo_pgsql/tests/gh21869.phpt b/ext/pdo_pgsql/tests/gh21869.phpt new file mode 100644 index 000000000000..2c64b6a674f6 --- /dev/null +++ b/ext/pdo_pgsql/tests/gh21869.phpt @@ -0,0 +1,46 @@ +--TEST-- +GH-21869 pdo_pgsql: DEALLOCATE failure in destructor must not poison the enclosing transaction +--EXTENSIONS-- +pdo +pdo_pgsql +--SKIPIF-- +getAttribute(PDO::ATTR_CLIENT_VERSION), '17', '>=')) { + die('skip libpq >= 17 uses PQclosePrepared instead of the DEALLOCATE query'); +} +?> +--FILE-- +setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); +$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false); + +$pdo->exec('CREATE TABLE gh21869 (a integer not null)'); + +$pdo->beginTransaction(); +$stmt = $pdo->prepare('INSERT INTO gh21869 (a) VALUES (?)'); +$stmt->execute([1]); + +foreach ($pdo->query('SELECT name FROM pg_prepared_statements') as $row) { + $pdo->exec('DEALLOCATE ' . $row['name']); +} + +unset($stmt); + +$pdo->commit(); + +var_dump((int) $pdo->query('SELECT count(*) FROM gh21869')->fetchColumn()); +?> +--CLEAN-- +exec('DROP TABLE IF EXISTS gh21869'); +?> +--EXPECT-- +int(1) From d7be0d58a397a945d092f15baca72e180fbf9251 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 25 Apr 2026 00:03:02 +0100 Subject: [PATCH 2/3] savepoint unsupported --- ext/pdo_pgsql/pgsql_statement.c | 38 ++++++--------------------------- 1 file changed, 7 insertions(+), 31 deletions(-) diff --git a/ext/pdo_pgsql/pgsql_statement.c b/ext/pdo_pgsql/pgsql_statement.c index e1581602196d..09ee64252e43 100644 --- a/ext/pdo_pgsql/pgsql_statement.c +++ b/ext/pdo_pgsql/pgsql_statement.c @@ -81,40 +81,16 @@ static int pgsql_stmt_dtor(pdo_stmt_t *stmt) char *q = NULL; PGTransactionStatusType tstatus = PQtransactionStatus(H->server); - switch (tstatus) { - case PQTRANS_ACTIVE: - case PQTRANS_INERROR: - case PQTRANS_UNKNOWN: - res = NULL; - break; - case PQTRANS_INTRANS: - spprintf(&q, 0, - "SAVEPOINT pdo_pgsql_deallocate_%s;" - "DEALLOCATE %s;" - "RELEASE SAVEPOINT pdo_pgsql_deallocate_%s", - S->stmt_name, - S->stmt_name, - S->stmt_name); - res = PQexec(H->server, q); - efree(q); - if (res && PQresultStatus(res) != PGRES_COMMAND_OK) { - spprintf(&q, 0, - "ROLLBACK TO SAVEPOINT pdo_pgsql_deallocate_%s;" - "RELEASE SAVEPOINT pdo_pgsql_deallocate_%s", - S->stmt_name, - S->stmt_name); - PGresult *rollres; - PQclear(res); - rollres = PQexec(H->server, q); - res = rollres; - efree(q); - } - break; - default: + if (tstatus == PQTRANS_IDLE) { spprintf(&q, 0, "DEALLOCATE %s", S->stmt_name); res = PQexec(H->server, q); efree(q); - break; + } else { + /* Inside a transaction, already aborted, or connection is unusable: + * skip DEALLOCATE. On servers that reject it (e.g. Aurora DSQL) it + * would poison the transaction (GH-21869); on any server the server + * frees the prepared statement when the transaction ends. */ + res = NULL; } #else res = PQclosePrepared(H->server, S->stmt_name); From 7016e7182117b5d80d079b2a8b2319639bd4093e Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sun, 26 Apr 2026 16:38:30 +0100 Subject: [PATCH 3/3] comment fix --- ext/pdo_pgsql/pgsql_statement.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ext/pdo_pgsql/pgsql_statement.c b/ext/pdo_pgsql/pgsql_statement.c index 09ee64252e43..cec83ce8b005 100644 --- a/ext/pdo_pgsql/pgsql_statement.c +++ b/ext/pdo_pgsql/pgsql_statement.c @@ -86,10 +86,9 @@ static int pgsql_stmt_dtor(pdo_stmt_t *stmt) res = PQexec(H->server, q); efree(q); } else { - /* Inside a transaction, already aborted, or connection is unusable: - * skip DEALLOCATE. On servers that reject it (e.g. Aurora DSQL) it - * would poison the transaction (GH-21869); on any server the server - * frees the prepared statement when the transaction ends. */ + /* Outside PQTRANS_IDLE, skip DEALLOCATE: on libpq < 17 some servers + * (e.g. Aurora DSQL) reject it and poison the tx (GH-21869). The + * statement is session-scoped, so it's freed on disconnect. */ res = NULL; } #else