Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 361844f

Browse files
committed
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
1 parent f510412 commit 361844f

File tree

2 files changed

+39
-9
lines changed

2 files changed

+39
-9
lines changed

src/backend/executor/spi.c

+33-9
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,16 @@
3636
#include "utils/typcache.h"
3737

3838

39+
/*
40+
* These global variables are part of the API for various SPI functions
41+
* (a horrible API choice, but it's too late now). To reduce the risk of
42+
* interference between different SPI callers, we save and restore them
43+
* when entering/exiting a SPI nesting level.
44+
*/
3945
uint64 SPI_processed = 0;
4046
Oid SPI_lastoid = InvalidOid;
4147
SPITupleTable *SPI_tuptable = NULL;
42-
int SPI_result;
48+
int SPI_result = 0;
4349

4450
static _SPI_connection *_SPI_stack = NULL;
4551
static _SPI_connection *_SPI_current = NULL;
@@ -132,6 +138,10 @@ SPI_connect_ext(int options)
132138
_SPI_current->queryEnv = NULL;
133139
_SPI_current->atomic = (options & SPI_OPT_NONATOMIC ? false : true);
134140
_SPI_current->internal_xact = false;
141+
_SPI_current->outer_processed = SPI_processed;
142+
_SPI_current->outer_lastoid = SPI_lastoid;
143+
_SPI_current->outer_tuptable = SPI_tuptable;
144+
_SPI_current->outer_result = SPI_result;
135145

136146
/*
137147
* Create memory contexts for this procedure
@@ -154,6 +164,15 @@ SPI_connect_ext(int options)
154164
/* ... and switch to procedure's context */
155165
_SPI_current->savedcxt = MemoryContextSwitchTo(_SPI_current->procCxt);
156166

167+
/*
168+
* Reset API global variables so that current caller cannot accidentally
169+
* depend on state of an outer caller.
170+
*/
171+
SPI_processed = 0;
172+
SPI_lastoid = InvalidOid;
173+
SPI_tuptable = NULL;
174+
SPI_result = 0;
175+
157176
return SPI_OK_CONNECT;
158177
}
159178

@@ -176,12 +195,13 @@ SPI_finish(void)
176195
_SPI_current->procCxt = NULL;
177196

178197
/*
179-
* Reset result variables, especially SPI_tuptable which is probably
198+
* Restore outer API variables, especially SPI_tuptable which is probably
180199
* pointing at a just-deleted tuptable
181200
*/
182-
SPI_processed = 0;
183-
SPI_lastoid = InvalidOid;
184-
SPI_tuptable = NULL;
201+
SPI_processed = _SPI_current->outer_processed;
202+
SPI_lastoid = _SPI_current->outer_lastoid;
203+
SPI_tuptable = _SPI_current->outer_tuptable;
204+
SPI_result = _SPI_current->outer_result;
185205

186206
/* Exit stack level */
187207
_SPI_connected--;
@@ -274,9 +294,11 @@ SPICleanup(void)
274294
{
275295
_SPI_current = NULL;
276296
_SPI_connected = -1;
297+
/* Reset API global variables, too */
277298
SPI_processed = 0;
278299
SPI_lastoid = InvalidOid;
279300
SPI_tuptable = NULL;
301+
SPI_result = 0;
280302
}
281303

282304
/*
@@ -336,18 +358,20 @@ AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)
336358
}
337359

338360
/*
339-
* Pop the stack entry and reset global variables. Unlike
361+
* Restore outer global variables and pop the stack entry. Unlike
340362
* SPI_finish(), we don't risk switching to memory contexts that might
341363
* be already gone.
342364
*/
365+
SPI_processed = connection->outer_processed;
366+
SPI_lastoid = connection->outer_lastoid;
367+
SPI_tuptable = connection->outer_tuptable;
368+
SPI_result = connection->outer_result;
369+
343370
_SPI_connected--;
344371
if (_SPI_connected < 0)
345372
_SPI_current = NULL;
346373
else
347374
_SPI_current = &(_SPI_stack[_SPI_connected]);
348-
SPI_processed = 0;
349-
SPI_lastoid = InvalidOid;
350-
SPI_tuptable = NULL;
351375
}
352376

353377
if (found && isCommit)

src/include/executor/spi_priv.h

+6
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ typedef struct
4242
* transactions */
4343
bool internal_xact; /* SPI-managed transaction boundary, skip
4444
* cleanup */
45+
46+
/* saved values of API global variables for previous nesting level */
47+
uint64 outer_processed;
48+
Oid outer_lastoid;
49+
SPITupleTable *outer_tuptable;
50+
int outer_result;
4551
} _SPI_connection;
4652

4753
/*

0 commit comments

Comments
 (0)