diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index f9d257d1a1..f25b857045 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/async.c,v 1.115 2004/08/29 05:06:41 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/async.c,v 1.116 2004/09/06 23:32:54 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -630,6 +630,9 @@ AtSubStart_Notify(void) upperPendingNotifies = lcons(pendingNotifies, upperPendingNotifies); + Assert(list_length(upperPendingNotifies) == + GetCurrentTransactionNestLevel() - 1); + pendingNotifies = NIL; MemoryContextSwitchTo(old_cxt); @@ -648,6 +651,9 @@ AtSubCommit_Notify(void) parentPendingNotifies = (List *) linitial(upperPendingNotifies); upperPendingNotifies = list_delete_first(upperPendingNotifies); + Assert(list_length(upperPendingNotifies) == + GetCurrentTransactionNestLevel() - 2); + /* * We could try to eliminate duplicates here, but it seems not * worthwhile. @@ -661,13 +667,23 @@ AtSubCommit_Notify(void) void AtSubAbort_Notify(void) { + int my_level = GetCurrentTransactionNestLevel(); + /* * All we have to do is pop the stack --- the notifies made in this * subxact are no longer interesting, and the space will be freed when * CurTransactionContext is recycled. + * + * This routine could be called more than once at a given nesting level + * if there is trouble during subxact abort. Avoid dumping core by + * using GetCurrentTransactionNestLevel as the indicator of how far + * we need to prune the list. */ - pendingNotifies = (List *) linitial(upperPendingNotifies); - upperPendingNotifies = list_delete_first(upperPendingNotifies); + while (list_length(upperPendingNotifies) > my_level - 2) + { + pendingNotifies = (List *) linitial(upperPendingNotifies); + upperPendingNotifies = list_delete_first(upperPendingNotifies); + } } /* diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 7e73f6b000..18260e6272 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.168 2004/08/29 05:06:41 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.169 2004/09/06 23:32:54 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1669,8 +1669,11 @@ ltrmark:; * state data; each subtransaction level that modifies that state first * saves a copy, which we use to restore the state if we abort. * - * numpushed and numalloc keep control of allocation and storage in the above - * stacks. numpushed is essentially the current subtransaction nesting depth. + * We use GetCurrentTransactionNestLevel() to determine the correct array + * index in these stacks. numalloc is the number of allocated entries in + * each stack. (By not keeping our own stack pointer, we can avoid trouble + * in cases where errors during subxact abort cause multiple invocations + * of DeferredTriggerEndSubXact() at the same nesting depth.) * * XXX We need to be able to save the per-event data in a file if it grows too * large. @@ -1742,7 +1745,6 @@ typedef struct DeferredTriggersData DeferredTriggerEvent *tail_stack; DeferredTriggerEvent *imm_stack; DeferredTriggerState *state_stack; - int numpushed; int numalloc; } DeferredTriggersData; @@ -2165,7 +2167,6 @@ DeferredTriggerBeginXact(void) deferredTriggers->imm_stack = NULL; deferredTriggers->state_stack = NULL; deferredTriggers->numalloc = 0; - deferredTriggers->numpushed = 0; } @@ -2251,6 +2252,8 @@ DeferredTriggerAbortXact(void) void DeferredTriggerBeginSubXact(void) { + int my_level = GetCurrentTransactionNestLevel(); + /* * Ignore call if the transaction is in aborted state. */ @@ -2260,7 +2263,7 @@ DeferredTriggerBeginSubXact(void) /* * Allocate more space in the stacks if needed. */ - if (deferredTriggers->numpushed == deferredTriggers->numalloc) + while (my_level >= deferredTriggers->numalloc) { if (deferredTriggers->numalloc == 0) { @@ -2282,32 +2285,28 @@ DeferredTriggerBeginSubXact(void) else { /* repalloc will keep the stacks in the same context */ - deferredTriggers->numalloc *= 2; + int new_alloc = deferredTriggers->numalloc * 2; deferredTriggers->tail_stack = (DeferredTriggerEvent *) repalloc(deferredTriggers->tail_stack, - deferredTriggers->numalloc * sizeof(DeferredTriggerEvent)); + new_alloc * sizeof(DeferredTriggerEvent)); deferredTriggers->imm_stack = (DeferredTriggerEvent *) repalloc(deferredTriggers->imm_stack, - deferredTriggers->numalloc * sizeof(DeferredTriggerEvent)); + new_alloc * sizeof(DeferredTriggerEvent)); deferredTriggers->state_stack = (DeferredTriggerState *) repalloc(deferredTriggers->state_stack, - deferredTriggers->numalloc * sizeof(DeferredTriggerState)); + new_alloc * sizeof(DeferredTriggerState)); + deferredTriggers->numalloc = new_alloc; } } /* - * Push the current list position into the stack and reset the - * pointer. + * Push the current information into the stack. */ - deferredTriggers->tail_stack[deferredTriggers->numpushed] = - deferredTriggers->tail_thisxact; - deferredTriggers->imm_stack[deferredTriggers->numpushed] = - deferredTriggers->events_imm; + deferredTriggers->tail_stack[my_level] = deferredTriggers->tail_thisxact; + deferredTriggers->imm_stack[my_level] = deferredTriggers->events_imm; /* State is not saved until/unless changed */ - deferredTriggers->state_stack[deferredTriggers->numpushed] = NULL; - - deferredTriggers->numpushed++; + deferredTriggers->state_stack[my_level] = NULL; } /* @@ -2318,6 +2317,7 @@ DeferredTriggerBeginSubXact(void) void DeferredTriggerEndSubXact(bool isCommit) { + int my_level = GetCurrentTransactionNestLevel(); DeferredTriggerState state; /* @@ -2327,18 +2327,18 @@ DeferredTriggerEndSubXact(bool isCommit) return; /* - * Move back the "top of the stack." + * Pop the prior state if needed. */ - Assert(deferredTriggers->numpushed > 0); - - deferredTriggers->numpushed--; + Assert(my_level < deferredTriggers->numalloc); if (isCommit) { /* If we saved a prior state, we don't need it anymore */ - state = deferredTriggers->state_stack[deferredTriggers->numpushed]; + state = deferredTriggers->state_stack[my_level]; if (state != NULL) pfree(state); + /* this avoids double pfree if error later: */ + deferredTriggers->state_stack[my_level] = NULL; } else { @@ -2346,9 +2346,9 @@ DeferredTriggerEndSubXact(bool isCommit) * Aborting --- restore the pointers from the stacks. */ deferredTriggers->tail_thisxact = - deferredTriggers->tail_stack[deferredTriggers->numpushed]; + deferredTriggers->tail_stack[my_level]; deferredTriggers->events_imm = - deferredTriggers->imm_stack[deferredTriggers->numpushed]; + deferredTriggers->imm_stack[my_level]; /* * Cleanup the head and the tail of the list. @@ -2367,12 +2367,14 @@ DeferredTriggerEndSubXact(bool isCommit) * Restore the trigger state. If the saved state is NULL, then * this subxact didn't save it, so it doesn't need restoring. */ - state = deferredTriggers->state_stack[deferredTriggers->numpushed]; + state = deferredTriggers->state_stack[my_level]; if (state != NULL) { pfree(deferredTriggers->state); deferredTriggers->state = state; } + /* this avoids double pfree if error later: */ + deferredTriggers->state_stack[my_level] = NULL; } } @@ -2457,6 +2459,8 @@ DeferredTriggerStateAddItem(DeferredTriggerState state, void DeferredTriggerSetState(ConstraintsSetStmt *stmt) { + int my_level = GetCurrentTransactionNestLevel(); + /* * Ignore call if we aren't in a transaction. */ @@ -2468,10 +2472,10 @@ DeferredTriggerSetState(ConstraintsSetStmt *stmt) * already, save it so it can be restored if the subtransaction * aborts. */ - if (deferredTriggers->numpushed > 0 && - deferredTriggers->state_stack[deferredTriggers->numpushed - 1] == NULL) + if (my_level > 1 && + deferredTriggers->state_stack[my_level] == NULL) { - deferredTriggers->state_stack[deferredTriggers->numpushed - 1] = + deferredTriggers->state_stack[my_level] = DeferredTriggerStateCopy(deferredTriggers->state); } diff --git a/src/backend/storage/ipc/sinval.c b/src/backend/storage/ipc/sinval.c index 830d45169a..5c4db3da80 100644 --- a/src/backend/storage/ipc/sinval.c +++ b/src/backend/storage/ipc/sinval.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/ipc/sinval.c,v 1.72 2004/08/29 05:06:48 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/storage/ipc/sinval.c,v 1.73 2004/09/06 23:33:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1059,8 +1059,14 @@ XidCacheRemoveRunningXids(TransactionId xid, int nxids, TransactionId *xids) break; } } - /* We should have found it, unless the cache has overflowed */ - Assert(j >= 0 || MyProc->subxids.overflowed); + /* + * Ordinarily we should have found it, unless the cache has overflowed. + * However it's also possible for this routine to be invoked multiple + * times for the same subtransaction, in case of an error during + * AbortSubTransaction. So instead of Assert, emit a debug warning. + */ + if (j < 0 && !MyProc->subxids.overflowed) + elog(WARNING, "did not find subXID %u in MyProc", anxid); } for (j = MyProc->subxids.nxids - 1; j >= 0; j--) @@ -1071,8 +1077,9 @@ XidCacheRemoveRunningXids(TransactionId xid, int nxids, TransactionId *xids) break; } } - /* We should have found it, unless the cache has overflowed */ - Assert(j >= 0 || MyProc->subxids.overflowed); + /* Ordinarily we should have found it, unless the cache has overflowed */ + if (j < 0 && !MyProc->subxids.overflowed) + elog(WARNING, "did not find subXID %u in MyProc", xid); LWLockRelease(SInvalLock); } diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c index 3c85b05dee..8dd806bb0d 100644 --- a/src/backend/utils/cache/inval.c +++ b/src/backend/utils/cache/inval.c @@ -80,12 +80,13 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/cache/inval.c,v 1.66 2004/08/29 05:06:50 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/cache/inval.c,v 1.67 2004/09/06 23:33:48 tgl Exp $ * *------------------------------------------------------------------------- */ #include "postgres.h" +#include "access/xact.h" #include "catalog/catalog.h" #include "miscadmin.h" #include "storage/sinval.h" @@ -139,6 +140,9 @@ typedef struct TransInvalidationInfo /* Back link to parent transaction's info */ struct TransInvalidationInfo *parent; + /* Subtransaction nesting depth */ + int my_level; + /* head of current-command event list */ InvalidationListHeader CurrentCmdInvalidMsgs; @@ -603,6 +607,7 @@ AtStart_Inval(void) transInvalInfo = (TransInvalidationInfo *) MemoryContextAllocZero(TopTransactionContext, sizeof(TransInvalidationInfo)); + transInvalInfo->my_level = GetCurrentTransactionNestLevel(); } /* @@ -619,6 +624,7 @@ AtSubStart_Inval(void) MemoryContextAllocZero(TopTransactionContext, sizeof(TransInvalidationInfo)); myInfo->parent = transInvalInfo; + myInfo->my_level = GetCurrentTransactionNestLevel(); transInvalInfo = myInfo; } @@ -649,11 +655,11 @@ AtSubStart_Inval(void) void AtEOXact_Inval(bool isCommit) { - /* Must be at top of stack */ - Assert(transInvalInfo != NULL && transInvalInfo->parent == NULL); - if (isCommit) { + /* Must be at top of stack */ + Assert(transInvalInfo != NULL && transInvalInfo->parent == NULL); + /* * Relcache init file invalidation requires processing both before * and after we send the SI messages. However, we need not do @@ -671,8 +677,11 @@ AtEOXact_Inval(bool isCommit) if (transInvalInfo->RelcacheInitFileInval) RelationCacheInitFileInvalidate(false); } - else + else if (transInvalInfo != NULL) { + /* Must be at top of stack */ + Assert(transInvalInfo->parent == NULL); + ProcessInvalidationMessages(&transInvalInfo->PriorCmdInvalidMsgs, LocalExecuteInvalidationMessage); } @@ -696,18 +705,21 @@ AtEOXact_Inval(bool isCommit) * * In any case, pop the transaction stack. We need not physically free memory * here, since CurTransactionContext is about to be emptied anyway - * (if aborting). + * (if aborting). Beware of the possibility of aborting the same nesting + * level twice, though. */ void AtEOSubXact_Inval(bool isCommit) { + int my_level = GetCurrentTransactionNestLevel(); TransInvalidationInfo *myInfo = transInvalInfo; - /* Must be at non-top of stack */ - Assert(myInfo != NULL && myInfo->parent != NULL); - if (isCommit) { + /* Must be at non-top of stack */ + Assert(myInfo != NULL && myInfo->parent != NULL); + Assert(myInfo->my_level == my_level); + /* If CurrentCmdInvalidMsgs still has anything, fix it */ CommandEndInvalidationMessages(); @@ -718,18 +730,27 @@ AtEOSubXact_Inval(bool isCommit) /* Pending relcache inval becomes parent's problem too */ if (myInfo->RelcacheInitFileInval) myInfo->parent->RelcacheInitFileInval = true; + + /* Pop the transaction state stack */ + transInvalInfo = myInfo->parent; + + /* Need not free anything else explicitly */ + pfree(myInfo); } - else + else if (myInfo != NULL && myInfo->my_level == my_level) { + /* Must be at non-top of stack */ + Assert(myInfo->parent != NULL); + ProcessInvalidationMessages(&myInfo->PriorCmdInvalidMsgs, LocalExecuteInvalidationMessage); + + /* Pop the transaction state stack */ + transInvalInfo = myInfo->parent; + + /* Need not free anything else explicitly */ + pfree(myInfo); } - - /* Pop the transaction state stack */ - transInvalInfo = myInfo->parent; - - /* Need not free anything else explicitly */ - pfree(myInfo); } /*