Skip to content

Commit 12d768e

Browse files
tglsfdctglsfdc
committed
Don't use static storage for SaveTransactionCharacteristics().
This is pretty queasy-making on general principles, and the more so once you notice that CommitTransactionCommand() is actually stomping on the values saved by _SPI_commit(). It's okay as long as the active values didn't change during HoldPinnedPortals(); but that's a larger assumption than I think we want to make, especially since the fix is so simple. Discussion: https://postgr.es/m/1533956.1645731245@sss.pgh.pa.us
1 parent 2e51781 commit 12d768e

File tree

3 files changed

+32
-28
lines changed

3 files changed

+32
-28
lines changed

‎src/backend/access/transam/xact.c‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2983,24 +2983,20 @@ StartTransactionCommand(void)
29832983
* GUC system resets the characteristics at transaction end, so for example
29842984
* just skipping the reset in StartTransaction() won't work.)
29852985
*/
2986-
static int save_XactIsoLevel;
2987-
static bool save_XactReadOnly;
2988-
static bool save_XactDeferrable;
2989-
29902986
void
2991-
SaveTransactionCharacteristics(void)
2987+
SaveTransactionCharacteristics(SavedTransactionCharacteristics *s)
29922988
{
2993-
save_XactIsoLevel = XactIsoLevel;
2994-
save_XactReadOnly = XactReadOnly;
2995-
save_XactDeferrable = XactDeferrable;
2989+
s->save_XactIsoLevel = XactIsoLevel;
2990+
s->save_XactReadOnly = XactReadOnly;
2991+
s->save_XactDeferrable = XactDeferrable;
29962992
}
29972993

29982994
void
2999-
RestoreTransactionCharacteristics(void)
2995+
RestoreTransactionCharacteristics(const SavedTransactionCharacteristics *s)
30002996
{
3001-
XactIsoLevel = save_XactIsoLevel;
3002-
XactReadOnly = save_XactReadOnly;
3003-
XactDeferrable = save_XactDeferrable;
2997+
XactIsoLevel = s->save_XactIsoLevel;
2998+
XactReadOnly = s->save_XactReadOnly;
2999+
XactDeferrable = s->save_XactDeferrable;
30043000
}
30053001

30063002

@@ -3011,9 +3007,9 @@ void
30113007
CommitTransactionCommand(void)
30123008
{
30133009
TransactionState s = CurrentTransactionState;
3010+
SavedTransactionCharacteristics savetc;
30143011

3015-
if (s->chain)
3016-
SaveTransactionCharacteristics();
3012+
SaveTransactionCharacteristics(&savetc);
30173013

30183014
switch (s->blockState)
30193015
{
@@ -3071,7 +3067,7 @@ CommitTransactionCommand(void)
30713067
StartTransaction();
30723068
s->blockState = TBLOCK_INPROGRESS;
30733069
s->chain = false;
3074-
RestoreTransactionCharacteristics();
3070+
RestoreTransactionCharacteristics(&savetc);
30753071
}
30763072
break;
30773073

@@ -3097,7 +3093,7 @@ CommitTransactionCommand(void)
30973093
StartTransaction();
30983094
s->blockState = TBLOCK_INPROGRESS;
30993095
s->chain = false;
3100-
RestoreTransactionCharacteristics();
3096+
RestoreTransactionCharacteristics(&savetc);
31013097
}
31023098
break;
31033099

@@ -3115,7 +3111,7 @@ CommitTransactionCommand(void)
31153111
StartTransaction();
31163112
s->blockState = TBLOCK_INPROGRESS;
31173113
s->chain = false;
3118-
RestoreTransactionCharacteristics();
3114+
RestoreTransactionCharacteristics(&savetc);
31193115
}
31203116
break;
31213117

@@ -3182,7 +3178,7 @@ CommitTransactionCommand(void)
31823178
StartTransaction();
31833179
s->blockState = TBLOCK_INPROGRESS;
31843180
s->chain = false;
3185-
RestoreTransactionCharacteristics();
3181+
RestoreTransactionCharacteristics(&savetc);
31863182
}
31873183
}
31883184
else if (s->blockState == TBLOCK_PREPARE)

‎src/backend/executor/spi.c‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ static void
228228
_SPI_commit(bool chain)
229229
{
230230
MemoryContext oldcontext = CurrentMemoryContext;
231+
SavedTransactionCharacteristics savetc;
231232

232233
/*
233234
* Complain if we are in a context that doesn't permit transaction
@@ -255,9 +256,8 @@ _SPI_commit(bool chain)
255256
(errcode(ERRCODE_INVALID_TRANSACTION_TERMINATION),
256257
errmsg("cannot commit while a subtransaction is active")));
257258

258-
/* XXX this ain't re-entrant enough for my taste */
259259
if (chain)
260-
SaveTransactionCharacteristics();
260+
SaveTransactionCharacteristics(&savetc);
261261

262262
/* Catch any error occurring during the COMMIT */
263263
PG_TRY();
@@ -281,7 +281,7 @@ _SPI_commit(bool chain)
281281
/* Immediately start a new transaction */
282282
StartTransactionCommand();
283283
if (chain)
284-
RestoreTransactionCharacteristics();
284+
RestoreTransactionCharacteristics(&savetc);
285285

286286
MemoryContextSwitchTo(oldcontext);
287287

@@ -305,7 +305,7 @@ _SPI_commit(bool chain)
305305
/* ... and start a new one */
306306
StartTransactionCommand();
307307
if (chain)
308-
RestoreTransactionCharacteristics();
308+
RestoreTransactionCharacteristics(&savetc);
309309

310310
MemoryContextSwitchTo(oldcontext);
311311

@@ -333,6 +333,7 @@ static void
333333
_SPI_rollback(bool chain)
334334
{
335335
MemoryContext oldcontext = CurrentMemoryContext;
336+
SavedTransactionCharacteristics savetc;
336337

337338
/* see under SPI_commit() */
338339
if (_SPI_current->atomic)
@@ -346,9 +347,8 @@ _SPI_rollback(bool chain)
346347
(errcode(ERRCODE_INVALID_TRANSACTION_TERMINATION),
347348
errmsg("cannot roll back while a subtransaction is active")));
348349

349-
/* XXX this ain't re-entrant enough for my taste */
350350
if (chain)
351-
SaveTransactionCharacteristics();
351+
SaveTransactionCharacteristics(&savetc);
352352

353353
/* Catch any error occurring during the ROLLBACK */
354354
PG_TRY();
@@ -373,7 +373,7 @@ _SPI_rollback(bool chain)
373373
/* Immediately start a new transaction */
374374
StartTransactionCommand();
375375
if (chain)
376-
RestoreTransactionCharacteristics();
376+
RestoreTransactionCharacteristics(&savetc);
377377

378378
MemoryContextSwitchTo(oldcontext);
379379

@@ -398,7 +398,7 @@ _SPI_rollback(bool chain)
398398
/* ... and start a new one */
399399
StartTransactionCommand();
400400
if (chain)
401-
RestoreTransactionCharacteristics();
401+
RestoreTransactionCharacteristics(&savetc);
402402

403403
MemoryContextSwitchTo(oldcontext);
404404

‎src/include/access/xact.h‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,14 @@ typedef enum
135135
typedef void (*SubXactCallback) (SubXactEvent event, SubTransactionId mySubid,
136136
SubTransactionId parentSubid, void *arg);
137137

138+
/* Data structure for Save/RestoreTransactionCharacteristics */
139+
typedef struct SavedTransactionCharacteristics
140+
{
141+
int save_XactIsoLevel;
142+
bool save_XactReadOnly;
143+
bool save_XactDeferrable;
144+
} SavedTransactionCharacteristics;
145+
138146

139147
/* ----------------
140148
* transaction-related XLOG entries
@@ -399,8 +407,8 @@ extern bool TransactionIdIsCurrentTransactionId(TransactionId xid);
399407
extern void CommandCounterIncrement(void);
400408
extern void ForceSyncCommit(void);
401409
extern void StartTransactionCommand(void);
402-
extern void SaveTransactionCharacteristics(void);
403-
extern void RestoreTransactionCharacteristics(void);
410+
extern void SaveTransactionCharacteristics(SavedTransactionCharacteristics *s);
411+
extern void RestoreTransactionCharacteristics(const SavedTransactionCharacteristics *s);
404412
extern void CommitTransactionCommand(void);
405413
extern void AbortCurrentTransaction(void);
406414
extern void BeginTransactionBlock(void);

0 commit comments

Comments
 (0)