mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-09-01 12:47:25 +02:00
Save/restore SPI's global variables in SPI_connect() and SPI_finish().
This patch removes two sources of interference between nominally independent functions when one SPI-using function calls another, perhaps without knowing that it does so. Chapman Flack pointed out that xml.c's query_to_xml_internal() expects SPI_tuptable and SPI_processed to stay valid across datatype output function calls; but it's possible that such a call could involve re-entrant use of SPI. It seems likely that there are similar hazards elsewhere, if not in the core code then in third-party SPI users. Previously SPI_finish() reset SPI's API globals to zeroes/nulls, which would typically make for a crash in such a situation. Restoring them to the values they had at SPI_connect() seems like a considerably more useful behavior, and it still meets the design goal of not leaving any dangling pointers to tuple tables of the function being exited. Also, cause SPI_connect() to reset these variables to zeroes/nulls after saving them. This prevents interference in the opposite direction: it's possible that a SPI-using function that's only ever been tested standalone contains assumptions that these variables start out as zeroes. That was the case as long as you were the outermost SPI user, but not so much for an inner user. Now it's consistent. Report and fix suggestion by Chapman Flack, actual patch by me. Back-patch to all supported branches. Discussion: https://postgr.es/m/9fa25bef-2e4f-1c32-22a4-3ad0723c4a17@anastigmatix.net
This commit is contained in:
parent
90fd3bfd17
commit
825f10fbda
@ -36,10 +36,16 @@
|
||||
#include "utils/typcache.h"
|
||||
|
||||
|
||||
/*
|
||||
* These global variables are part of the API for various SPI functions
|
||||
* (a horrible API choice, but it's too late now). To reduce the risk of
|
||||
* interference between different SPI callers, we save and restore them
|
||||
* when entering/exiting a SPI nesting level.
|
||||
*/
|
||||
uint64 SPI_processed = 0;
|
||||
Oid SPI_lastoid = InvalidOid;
|
||||
SPITupleTable *SPI_tuptable = NULL;
|
||||
int SPI_result;
|
||||
int SPI_result = 0;
|
||||
|
||||
static _SPI_connection *_SPI_stack = NULL;
|
||||
static _SPI_connection *_SPI_current = NULL;
|
||||
@ -132,6 +138,10 @@ SPI_connect_ext(int options)
|
||||
_SPI_current->queryEnv = NULL;
|
||||
_SPI_current->atomic = (options & SPI_OPT_NONATOMIC ? false : true);
|
||||
_SPI_current->internal_xact = false;
|
||||
_SPI_current->outer_processed = SPI_processed;
|
||||
_SPI_current->outer_lastoid = SPI_lastoid;
|
||||
_SPI_current->outer_tuptable = SPI_tuptable;
|
||||
_SPI_current->outer_result = SPI_result;
|
||||
|
||||
/*
|
||||
* Create memory contexts for this procedure
|
||||
@ -154,6 +164,15 @@ SPI_connect_ext(int options)
|
||||
/* ... and switch to procedure's context */
|
||||
_SPI_current->savedcxt = MemoryContextSwitchTo(_SPI_current->procCxt);
|
||||
|
||||
/*
|
||||
* Reset API global variables so that current caller cannot accidentally
|
||||
* depend on state of an outer caller.
|
||||
*/
|
||||
SPI_processed = 0;
|
||||
SPI_lastoid = InvalidOid;
|
||||
SPI_tuptable = NULL;
|
||||
SPI_result = 0;
|
||||
|
||||
return SPI_OK_CONNECT;
|
||||
}
|
||||
|
||||
@ -176,12 +195,13 @@ SPI_finish(void)
|
||||
_SPI_current->procCxt = NULL;
|
||||
|
||||
/*
|
||||
* Reset result variables, especially SPI_tuptable which is probably
|
||||
* Restore outer API variables, especially SPI_tuptable which is probably
|
||||
* pointing at a just-deleted tuptable
|
||||
*/
|
||||
SPI_processed = 0;
|
||||
SPI_lastoid = InvalidOid;
|
||||
SPI_tuptable = NULL;
|
||||
SPI_processed = _SPI_current->outer_processed;
|
||||
SPI_lastoid = _SPI_current->outer_lastoid;
|
||||
SPI_tuptable = _SPI_current->outer_tuptable;
|
||||
SPI_result = _SPI_current->outer_result;
|
||||
|
||||
/* Exit stack level */
|
||||
_SPI_connected--;
|
||||
@ -274,9 +294,11 @@ SPICleanup(void)
|
||||
{
|
||||
_SPI_current = NULL;
|
||||
_SPI_connected = -1;
|
||||
/* Reset API global variables, too */
|
||||
SPI_processed = 0;
|
||||
SPI_lastoid = InvalidOid;
|
||||
SPI_tuptable = NULL;
|
||||
SPI_result = 0;
|
||||
}
|
||||
|
||||
/*
|
||||
@ -336,18 +358,20 @@ AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)
|
||||
}
|
||||
|
||||
/*
|
||||
* Pop the stack entry and reset global variables. Unlike
|
||||
* Restore outer global variables and pop the stack entry. Unlike
|
||||
* SPI_finish(), we don't risk switching to memory contexts that might
|
||||
* be already gone.
|
||||
*/
|
||||
SPI_processed = connection->outer_processed;
|
||||
SPI_lastoid = connection->outer_lastoid;
|
||||
SPI_tuptable = connection->outer_tuptable;
|
||||
SPI_result = connection->outer_result;
|
||||
|
||||
_SPI_connected--;
|
||||
if (_SPI_connected < 0)
|
||||
_SPI_current = NULL;
|
||||
else
|
||||
_SPI_current = &(_SPI_stack[_SPI_connected]);
|
||||
SPI_processed = 0;
|
||||
SPI_lastoid = InvalidOid;
|
||||
SPI_tuptable = NULL;
|
||||
}
|
||||
|
||||
if (found && isCommit)
|
||||
|
@ -42,6 +42,12 @@ typedef struct
|
||||
* transactions */
|
||||
bool internal_xact; /* SPI-managed transaction boundary, skip
|
||||
* cleanup */
|
||||
|
||||
/* saved values of API global variables for previous nesting level */
|
||||
uint64 outer_processed;
|
||||
Oid outer_lastoid;
|
||||
SPITupleTable *outer_tuptable;
|
||||
int outer_result;
|
||||
} _SPI_connection;
|
||||
|
||||
/*
|
||||
|
Loading…
Reference in New Issue
Block a user