Fix GH-21869: pdo_pgsql DEALLOCATE in destructor can poison the enclosing transaction.#21870
Fix GH-21869: pdo_pgsql DEALLOCATE in destructor can poison the enclosing transaction.#21870devnexen wants to merge 3 commits intophp:PHP-8.4from
Conversation
…closing 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.
devnexen
changed the title
devnexen
changed the title
devnexen
changed the title
| * would poison the transaction (GH-21869); on any server the server | ||
| * frees the prepared statement when the transaction ends. */ |
There was a problem hiding this comment.
I believe the server will hang on to the prepared statement until the connection is dropped, so this could leak prepared statements
There was a problem hiding this comment.
Right, session-scoped not transaction-scoped, comment fixed. Skipping is still the lesser evil since on Aurora DSQL with libpq < 17, DEALLOCATE inside a tx silently
turns COMMIT into ROLLBACK. The leak is bounded to session lifetime. Lazy DEALLOCATE on next idle query as a follow-up if persistent connections make it worth the
complexity.
|
|
||
| if (tstatus == PQTRANS_IDLE) { | ||
| spprintf(&q, 0, "DEALLOCATE %s", S->stmt_name); | ||
| res = PQexec(H->server, q); |
There was a problem hiding this comment.
I think if this errors it should get bubbled up
There was a problem hiding this comment.
Dtor path, no return channel to userland. The pre-patch code already discarded the result the same way. Surfacing dtor cleanup failures across pdo_pgsql might be worth
doing, but as a separate PR.
2 participants
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.