Repository navigation
PostgreSQL: databasevm_step closes an SPI cursor after catching an error without a subtransaction abort (server crash) #71
Description
Activity
The savepoint wrappers in
src/postgresql/database_postgresql.cfollow the same pattern asdatabasevm_step: each one catches a PostgreSQL error withPG_CATCH, callsFlushErrorState()and returns an error code, without a real subtransaction abort in between.database_commit_savepoint/database_rollback_savepoint: ifReleaseCurrentSubTransaction()orRollbackAndReleaseCurrentSubTransaction()throws, the subtransaction may be left half-ended. The caller's resource owner and memory context are also never restored on that path. The success path restores them after the subtransaction ends; that is the logic Remove the PostgreSQL savepoint resource-owner depth limit #66 reworks.database_begin_savepoint: ifBeginInternalSubTransaction()fails partway, the error is flushed in the same way and execution continues.
These paths are less likely to be hit than the
databasevm_stepcrash above: ending a subtransaction rarely throws, and an error raised during an abort usually escalates by itself. Neither #66 nor any other open PR changes these handlers.Suggest widening this issue to every
PG_CATCH+FlushErrorState()site that continues without an abort, so the fix is applied consistently: re-throw, or go through a real subtransaction abort before touching portals, tuple tables or the resource owner.
databasevm_step(src/postgresql/database_postgresql.c) catches a PostgreSQL error inPG_CATCH, callsFlushErrorState(), and then keeps working with the SPI cursor:clear_fetch_batch(stmt)andclose_portal(stmt), which callsSPI_cursor_close. No subtransaction is rolled back in between.PostgreSQL releases resources such as LWLocks during (sub)transaction abort. Catching an error and continuing without one is only safe while the error was not raised while holding such a resource. When it was, the cleanup runs with the lock still held.
Observed crash
An assertion-enabled PostgreSQL 17 server terminated during a fragment apply:
The trigger was a serialization failure raised by SSI inside a heap scan, which happens while a buffer content lock is held. The backend aborted with signal 6 and the server restarted ("terminating any other active server processes").
A release build does not assert, but releasing a buffer pin while its content lock is still held is not safe either.
How it was reached
Applying pieces of one fragmented value from two concurrent
SERIALIZABLEtransactions, while the per-value advisory lock made the second one wait and read after the first committed. The lock is no longer taken underSERIALIZABLE(ce0557f), which is why the suite is green, but nothing prevents another 40001 or any other error raised under a buffer lock from reaching the same handler.Suggested fix
Wrap the SPI work in a subtransaction (
BeginInternalSubTransaction/RollbackAndReleaseCurrentSubTransaction) so that catching an error goes through a real abort, or re-throw and let the existing savepoints handle it. The cleanup after a caught error should not touch portals or tuple tables before the abort.