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

Commit 4c77cbb

Browse files
committed
PortalRun must guard against the possibility that the portal it's
running contains VACUUM or a similar command that will internally start and commit transactions. In such a case, the original caller values of CurrentMemoryContext and CurrentResourceOwner will point to objects that will be destroyed by the internal commit. We must restore these pointers to point to the newly-manufactured transaction context and resource owner, rather than possibly pointing to deleted memory. Also tweak xact.c so that AbortTransaction and AbortSubTransaction forcibly restore a sane value for CurrentResourceOwner, much as they have always done for CurrentMemoryContext. I'm not certain this is necessary but I'm feeling paranoid today. Responds to Sean Chittenden's bug report of 4-Oct.
1 parent ee7de3d commit 4c77cbb

File tree

2 files changed

+68
-10
lines changed

2 files changed

+68
-10
lines changed

src/backend/access/transam/xact.c

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.190 2004/09/16 20:17:16 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.191 2004/10/04 21:52:14 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -205,6 +205,7 @@ static void AssignSubTransactionId(TransactionState s);
205205
static void AbortTransaction(void);
206206
static void AtAbort_Memory(void);
207207
static void AtCleanup_Memory(void);
208+
static void AtAbort_ResourceOwner(void);
208209
static void AtCommit_LocalCache(void);
209210
static void AtCommit_Memory(void);
210211
static void AtStart_Cache(void);
@@ -229,6 +230,7 @@ static void PopTransaction(void);
229230

230231
static void AtSubAbort_Memory(void);
231232
static void AtSubCleanup_Memory(void);
233+
static void AtSubAbort_ResourceOwner(void);
232234
static void AtSubCommit_Memory(void);
233235
static void AtSubStart_Memory(void);
234236
static void AtSubStart_ResourceOwner(void);
@@ -1103,7 +1105,6 @@ AtAbort_Memory(void)
11031105
MemoryContextSwitchTo(TopMemoryContext);
11041106
}
11051107

1106-
11071108
/*
11081109
* AtSubAbort_Memory
11091110
*/
@@ -1115,6 +1116,33 @@ AtSubAbort_Memory(void)
11151116
MemoryContextSwitchTo(TopTransactionContext);
11161117
}
11171118

1119+
1120+
/*
1121+
* AtAbort_ResourceOwner
1122+
*/
1123+
static void
1124+
AtAbort_ResourceOwner(void)
1125+
{
1126+
/*
1127+
* Make sure we have a valid ResourceOwner, if possible (else it
1128+
* will be NULL, which is OK)
1129+
*/
1130+
CurrentResourceOwner = TopTransactionResourceOwner;
1131+
}
1132+
1133+
/*
1134+
* AtSubAbort_ResourceOwner
1135+
*/
1136+
static void
1137+
AtSubAbort_ResourceOwner(void)
1138+
{
1139+
TransactionState s = CurrentTransactionState;
1140+
1141+
/* Make sure we have a valid ResourceOwner */
1142+
CurrentResourceOwner = s->curTransactionOwner;
1143+
}
1144+
1145+
11181146
/*
11191147
* AtSubAbort_childXids
11201148
*/
@@ -1598,8 +1626,9 @@ AbortTransaction(void)
15981626
*/
15991627
s->state = TRANS_ABORT;
16001628

1601-
/* Make sure we are in a valid memory context */
1629+
/* Make sure we have a valid memory context and resource owner */
16021630
AtAbort_Memory();
1631+
AtAbort_ResourceOwner();
16031632

16041633
/*
16051634
* Reset user id which might have been changed transiently. We cannot
@@ -3338,6 +3367,7 @@ AbortSubTransaction(void)
33383367
* do abort processing
33393368
*/
33403369
AtSubAbort_Memory();
3370+
AtSubAbort_ResourceOwner();
33413371

33423372
/*
33433373
* We can skip all this stuff if the subxact failed before creating

src/backend/tcop/pquery.c

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/tcop/pquery.c,v 1.87 2004/09/13 20:07:05 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/tcop/pquery.c,v 1.88 2004/10/04 21:52:15 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -491,12 +491,14 @@ PortalRun(Portal portal, long count,
491491
char *completionTag)
492492
{
493493
bool result;
494+
ResourceOwner saveTopTransactionResourceOwner;
495+
MemoryContext saveTopTransactionContext;
494496
Portal saveActivePortal;
495497
Snapshot saveActiveSnapshot;
496498
ResourceOwner saveResourceOwner;
497499
MemoryContext savePortalContext;
498500
MemoryContext saveQueryContext;
499-
MemoryContext oldContext;
501+
MemoryContext saveMemoryContext;
500502

501503
AssertArg(PortalIsValid(portal));
502504

@@ -523,12 +525,26 @@ PortalRun(Portal portal, long count,
523525

524526
/*
525527
* Set up global portal context pointers.
528+
*
529+
* We have to play a special game here to support utility commands like
530+
* VACUUM and CLUSTER, which internally start and commit transactions.
531+
* When we are called to execute such a command, CurrentResourceOwner
532+
* will be pointing to the TopTransactionResourceOwner --- which will
533+
* be destroyed and replaced in the course of the internal commit and
534+
* restart. So we need to be prepared to restore it as pointing to
535+
* the exit-time TopTransactionResourceOwner. (Ain't that ugly? This
536+
* idea of internally starting whole new transactions is not good.)
537+
* CurrentMemoryContext has a similar problem, but the other pointers
538+
* we save here will be NULL or pointing to longer-lived objects.
526539
*/
540+
saveTopTransactionResourceOwner = TopTransactionResourceOwner;
541+
saveTopTransactionContext = TopTransactionContext;
527542
saveActivePortal = ActivePortal;
528543
saveActiveSnapshot = ActiveSnapshot;
529544
saveResourceOwner = CurrentResourceOwner;
530545
savePortalContext = PortalContext;
531546
saveQueryContext = QueryContext;
547+
saveMemoryContext = CurrentMemoryContext;
532548
PG_TRY();
533549
{
534550
ActivePortal = portal;
@@ -537,7 +553,7 @@ PortalRun(Portal portal, long count,
537553
PortalContext = PortalGetHeapMemory(portal);
538554
QueryContext = portal->queryContext;
539555

540-
oldContext = MemoryContextSwitchTo(PortalContext);
556+
MemoryContextSwitchTo(PortalContext);
541557

542558
switch (portal->strategy)
543559
{
@@ -620,21 +636,33 @@ PortalRun(Portal portal, long count,
620636
portal->status = PORTAL_FAILED;
621637

622638
/* Restore global vars and propagate error */
639+
if (saveMemoryContext == saveTopTransactionContext)
640+
MemoryContextSwitchTo(TopTransactionContext);
641+
else
642+
MemoryContextSwitchTo(saveMemoryContext);
623643
ActivePortal = saveActivePortal;
624644
ActiveSnapshot = saveActiveSnapshot;
625-
CurrentResourceOwner = saveResourceOwner;
645+
if (saveResourceOwner == saveTopTransactionResourceOwner)
646+
CurrentResourceOwner = TopTransactionResourceOwner;
647+
else
648+
CurrentResourceOwner = saveResourceOwner;
626649
PortalContext = savePortalContext;
627650
QueryContext = saveQueryContext;
628651

629652
PG_RE_THROW();
630653
}
631654
PG_END_TRY();
632655

633-
MemoryContextSwitchTo(oldContext);
634-
656+
if (saveMemoryContext == saveTopTransactionContext)
657+
MemoryContextSwitchTo(TopTransactionContext);
658+
else
659+
MemoryContextSwitchTo(saveMemoryContext);
635660
ActivePortal = saveActivePortal;
636661
ActiveSnapshot = saveActiveSnapshot;
637-
CurrentResourceOwner = saveResourceOwner;
662+
if (saveResourceOwner == saveTopTransactionResourceOwner)
663+
CurrentResourceOwner = TopTransactionResourceOwner;
664+
else
665+
CurrentResourceOwner = saveResourceOwner;
638666
PortalContext = savePortalContext;
639667
QueryContext = saveQueryContext;
640668

0 commit comments

Comments
 (0)