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
This commit is contained in:
Tom Lane 2022-02-28 12:54:12 -05:00
parent 2e517818f4
commit 12d768e704
3 changed files with 32 additions and 28 deletions

View File

@ -2983,24 +2983,20 @@ StartTransactionCommand(void)
* GUC system resets the characteristics at transaction end, so for example
* just skipping the reset in StartTransaction() won't work.)
*/
static int save_XactIsoLevel;
static bool save_XactReadOnly;
static bool save_XactDeferrable;
void
SaveTransactionCharacteristics(void)
SaveTransactionCharacteristics(SavedTransactionCharacteristics *s)
{
save_XactIsoLevel = XactIsoLevel;
save_XactReadOnly = XactReadOnly;
save_XactDeferrable = XactDeferrable;
s->save_XactIsoLevel = XactIsoLevel;
s->save_XactReadOnly = XactReadOnly;
s->save_XactDeferrable = XactDeferrable;
}
void
RestoreTransactionCharacteristics(void)
RestoreTransactionCharacteristics(const SavedTransactionCharacteristics *s)
{
XactIsoLevel = save_XactIsoLevel;
XactReadOnly = save_XactReadOnly;
XactDeferrable = save_XactDeferrable;
XactIsoLevel = s->save_XactIsoLevel;
XactReadOnly = s->save_XactReadOnly;
XactDeferrable = s->save_XactDeferrable;
}
@ -3011,9 +3007,9 @@ void
CommitTransactionCommand(void)
{
TransactionState s = CurrentTransactionState;
SavedTransactionCharacteristics savetc;
if (s->chain)
SaveTransactionCharacteristics();
SaveTransactionCharacteristics(&savetc);
switch (s->blockState)
{
@ -3071,7 +3067,7 @@ CommitTransactionCommand(void)
StartTransaction();
s->blockState = TBLOCK_INPROGRESS;
s->chain = false;
RestoreTransactionCharacteristics();
RestoreTransactionCharacteristics(&savetc);
}
break;
@ -3097,7 +3093,7 @@ CommitTransactionCommand(void)
StartTransaction();
s->blockState = TBLOCK_INPROGRESS;
s->chain = false;
RestoreTransactionCharacteristics();
RestoreTransactionCharacteristics(&savetc);
}
break;
@ -3115,7 +3111,7 @@ CommitTransactionCommand(void)
StartTransaction();
s->blockState = TBLOCK_INPROGRESS;
s->chain = false;
RestoreTransactionCharacteristics();
RestoreTransactionCharacteristics(&savetc);
}
break;
@ -3182,7 +3178,7 @@ CommitTransactionCommand(void)
StartTransaction();
s->blockState = TBLOCK_INPROGRESS;
s->chain = false;
RestoreTransactionCharacteristics();
RestoreTransactionCharacteristics(&savetc);
}
}
else if (s->blockState == TBLOCK_PREPARE)

View File

@ -228,6 +228,7 @@ static void
_SPI_commit(bool chain)
{
MemoryContext oldcontext = CurrentMemoryContext;
SavedTransactionCharacteristics savetc;
/*
* Complain if we are in a context that doesn't permit transaction
@ -255,9 +256,8 @@ _SPI_commit(bool chain)
(errcode(ERRCODE_INVALID_TRANSACTION_TERMINATION),
errmsg("cannot commit while a subtransaction is active")));
/* XXX this ain't re-entrant enough for my taste */
if (chain)
SaveTransactionCharacteristics();
SaveTransactionCharacteristics(&savetc);
/* Catch any error occurring during the COMMIT */
PG_TRY();
@ -281,7 +281,7 @@ _SPI_commit(bool chain)
/* Immediately start a new transaction */
StartTransactionCommand();
if (chain)
RestoreTransactionCharacteristics();
RestoreTransactionCharacteristics(&savetc);
MemoryContextSwitchTo(oldcontext);
@ -305,7 +305,7 @@ _SPI_commit(bool chain)
/* ... and start a new one */
StartTransactionCommand();
if (chain)
RestoreTransactionCharacteristics();
RestoreTransactionCharacteristics(&savetc);
MemoryContextSwitchTo(oldcontext);
@ -333,6 +333,7 @@ static void
_SPI_rollback(bool chain)
{
MemoryContext oldcontext = CurrentMemoryContext;
SavedTransactionCharacteristics savetc;
/* see under SPI_commit() */
if (_SPI_current->atomic)
@ -346,9 +347,8 @@ _SPI_rollback(bool chain)
(errcode(ERRCODE_INVALID_TRANSACTION_TERMINATION),
errmsg("cannot roll back while a subtransaction is active")));
/* XXX this ain't re-entrant enough for my taste */
if (chain)
SaveTransactionCharacteristics();
SaveTransactionCharacteristics(&savetc);
/* Catch any error occurring during the ROLLBACK */
PG_TRY();
@ -373,7 +373,7 @@ _SPI_rollback(bool chain)
/* Immediately start a new transaction */
StartTransactionCommand();
if (chain)
RestoreTransactionCharacteristics();
RestoreTransactionCharacteristics(&savetc);
MemoryContextSwitchTo(oldcontext);
@ -398,7 +398,7 @@ _SPI_rollback(bool chain)
/* ... and start a new one */
StartTransactionCommand();
if (chain)
RestoreTransactionCharacteristics();
RestoreTransactionCharacteristics(&savetc);
MemoryContextSwitchTo(oldcontext);

View File

@ -135,6 +135,14 @@ typedef enum
typedef void (*SubXactCallback) (SubXactEvent event, SubTransactionId mySubid,
SubTransactionId parentSubid, void *arg);
/* Data structure for Save/RestoreTransactionCharacteristics */
typedef struct SavedTransactionCharacteristics
{
int save_XactIsoLevel;
bool save_XactReadOnly;
bool save_XactDeferrable;
} SavedTransactionCharacteristics;
/* ----------------
* transaction-related XLOG entries
@ -399,8 +407,8 @@ extern bool TransactionIdIsCurrentTransactionId(TransactionId xid);
extern void CommandCounterIncrement(void);
extern void ForceSyncCommit(void);
extern void StartTransactionCommand(void);
extern void SaveTransactionCharacteristics(void);
extern void RestoreTransactionCharacteristics(void);
extern void SaveTransactionCharacteristics(SavedTransactionCharacteristics *s);
extern void RestoreTransactionCharacteristics(const SavedTransactionCharacteristics *s);
extern void CommitTransactionCommand(void);
extern void AbortCurrentTransaction(void);
extern void BeginTransactionBlock(void);