Skip to content

Commit d5605e2

Browse files
authored
[BABEL] Fix to prevent SPI_finish crash on partially-initialized SPI connection after OOM (#725) (#732)
SPI_connect_ext() increments _SPI_connected before allocating memory contexts (procCxt, execCxt). If AllocSetContextCreate fails with OOM between the counter increment and context allocation, the SPI stack is left in an inconsistent state as the counter says a connection exists but the memory contexts are NULL. In vanilla PostgreSQL this is harmless because SPI_finish is never called on error paths — AtEOXact_SPI safely resets the counter during transaction abort. However, Babelfish calls SPI_finish explicitly in error handling and cleanup paths to emulate SQL Server's error handling semantics. This exposes the inconsistent state and causes a crash in MemoryContextDelete(NULL). SPI_finish crashes on such entries because it unconditionally calls MemoryContextDelete on NULL pointers. SPI_finish_safe checks whether the connection is fully initialized. If so, it uses SPI_finish. If not, it safely cleans up any partial allocation and pops the stack. Extension PR: babelfish-for-postgresql/babelfish_extensions#4629 Task: BABEL-6349 Authored-by: Rucha Kulkarni ruchask@amazon.com
1 parent 8538b9b commit d5605e2

File tree

2 files changed

+50
-0
lines changed

2 files changed

+50
-0
lines changed

src/backend/executor/spi.c

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3457,3 +3457,52 @@ SPI_get_depth(void)
34573457
{
34583458
return _SPI_connected;
34593459
}
3460+
3461+
/*
3462+
* Modified version of SPI_finish for Babelfish, which safely handles
3463+
* the case where the current SPI connection was not fully initialized.
3464+
* For fully initialized connections, delegates to SPI_finish.
3465+
*/
3466+
int
3467+
SPI_finish_safe(void)
3468+
{
3469+
/* Check if there is an active SPI connection */
3470+
if (_SPI_current == NULL)
3471+
return SPI_ERROR_UNCONNECTED;
3472+
3473+
/* Fully initialized */
3474+
if (_SPI_current->procCxt != NULL &&
3475+
_SPI_current->execCxt != NULL)
3476+
{
3477+
return SPI_finish();
3478+
}
3479+
3480+
if (_SPI_current->internal_xact)
3481+
elog(FATAL, "SPI_finish_safe: unexpected state - partially initialized SPI connection should not have internal_xact set");
3482+
3483+
/* Clean up any partially allocated context */
3484+
if (_SPI_current->execCxt)
3485+
{
3486+
MemoryContextDelete(_SPI_current->execCxt);
3487+
_SPI_current->execCxt = NULL;
3488+
}
3489+
if (_SPI_current->procCxt)
3490+
{
3491+
MemoryContextDelete(_SPI_current->procCxt);
3492+
_SPI_current->procCxt = NULL;
3493+
}
3494+
3495+
/* Restore outer API variables */
3496+
SPI_processed = _SPI_current->outer_processed;
3497+
SPI_tuptable = _SPI_current->outer_tuptable;
3498+
SPI_result = _SPI_current->outer_result;
3499+
3500+
/* Pop the stack */
3501+
_SPI_connected--;
3502+
if (_SPI_connected < 0)
3503+
_SPI_current = NULL;
3504+
else
3505+
_SPI_current = &(_SPI_stack[_SPI_connected]);
3506+
3507+
return SPI_OK_FINISH;
3508+
}

src/include/executor/spi.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ extern PGDLLIMPORT int SPI_result;
113113
extern int SPI_connect(void);
114114
extern int SPI_connect_ext(int options);
115115
extern int SPI_finish(void);
116+
extern int SPI_finish_safe(void);
116117
extern int SPI_execute(const char *src, bool read_only, long tcount);
117118
extern int SPI_execute_extended(const char *src,
118119
const SPIExecuteOptions *options);

0 commit comments

Comments
 (0)