Several changes to reduce the probability of running out of memory during

AbortTransaction, which would lead to recursion and eventual PANIC exit
as illustrated in recent report from Jeff Davis.  First, in xact.c create
a special dedicated memory context for AbortTransaction to run in.  This
solves the problem as long as AbortTransaction doesn't need more than 32K
(or whatever other size we create the context with).  But in corner cases
it might.  Second, in trigger.c arrange to keep pending after-trigger event
records in separate contexts that can be freed near the beginning of
AbortTransaction, rather than having them persist until CleanupTransaction
as before.  Third, in portalmem.c arrange to free executor state data
earlier as well.  These two changes should result in backing off the
out-of-memory condition before AbortTransaction needs any significant
amount of memory, at least in typical cases such as memory overrun due
to too many trigger events or too big an executor hash table.  And all
the same for subtransaction abort too, of course.

src/backend/access/transam/xact.c

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.228 2006/11/05 22:42:07 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.229 2006/11/23 01:14:59 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -171,6 +171,12 @@ static TimestampTz stmtStartTimestamp;
  */
 static char *prepareGID;
 
+/*
+ * Private context for transaction-abort work --- we reserve space for this
+ * at startup to ensure that AbortTransaction and AbortSubTransaction can work
+ * when we've run out of memory.
+ */
+static MemoryContext TransactionAbortContext = NULL;
 
 /*
  * List of add-on start- and end-of-xact callbacks
@@ -555,6 +561,21 @@ AtStart_Memory(void)
 {
 	TransactionState s = CurrentTransactionState;
 
+	/*
+	 * If this is the first time through, create a private context for
+	 * AbortTransaction to work in.  By reserving some space now, we can
+	 * insulate AbortTransaction from out-of-memory scenarios.  Like
+	 * ErrorContext, we set it up with slow growth rate and a nonzero
+	 * minimum size, so that space will be reserved immediately.
+	 */
+	if (TransactionAbortContext == NULL)
+		TransactionAbortContext =
+			AllocSetContextCreate(TopMemoryContext,
+								  "TransactionAbortContext",
+								  32 * 1024,
+								  32 * 1024,
+								  32 * 1024);
+
 	/*
 	 * We shouldn't have a transaction context already.
 	 */
@@ -1086,20 +1107,15 @@ static void
 AtAbort_Memory(void)
 {
 	/*
-	 * Make sure we are in a valid context (not a child of
-	 * TopTransactionContext...).  Note that it is possible for this code to
-	 * be called when we aren't in a transaction at all; go directly to
-	 * TopMemoryContext in that case.
+	 * Switch into TransactionAbortContext, which should have some free
+	 * space even if nothing else does.  We'll work in this context until
+	 * we've finished cleaning up.
+	 *
+	 * It is barely possible to get here when we've not been able to create
+	 * TransactionAbortContext yet; if so use TopMemoryContext.
 	 */
-	if (TopTransactionContext != NULL)
-	{
-		MemoryContextSwitchTo(TopTransactionContext);
-
-		/*
-		 * We do not want to destroy the transaction's global state yet, so we
-		 * can't free any memory here.
-		 */
-	}
+	if (TransactionAbortContext != NULL)
+		MemoryContextSwitchTo(TransactionAbortContext);
 	else
 		MemoryContextSwitchTo(TopMemoryContext);
 }
@@ -1110,9 +1126,9 @@ AtAbort_Memory(void)
 static void
 AtSubAbort_Memory(void)
 {
-	Assert(TopTransactionContext != NULL);
+	Assert(TransactionAbortContext != NULL);
 
-	MemoryContextSwitchTo(TopTransactionContext);
+	MemoryContextSwitchTo(TransactionAbortContext);
 }
 
 
@@ -1272,13 +1288,19 @@ RecordSubTransactionAbort(void)
 static void
 AtCleanup_Memory(void)
 {
+	Assert(CurrentTransactionState->parent == NULL);
+
 	/*
 	 * Now that we're "out" of a transaction, have the system allocate things
 	 * in the top memory context instead of per-transaction contexts.
 	 */
 	MemoryContextSwitchTo(TopMemoryContext);
 
-	Assert(CurrentTransactionState->parent == NULL);
+	/*
+	 * Clear the special abort context for next time.
+	 */
+	if (TransactionAbortContext != NULL)
+		MemoryContextResetAndDeleteChildren(TransactionAbortContext);
 
 	/*
 	 * Release all transaction-local memory.
@@ -1310,6 +1332,12 @@ AtSubCleanup_Memory(void)
 	MemoryContextSwitchTo(s->parent->curTransactionContext);
 	CurTransactionContext = s->parent->curTransactionContext;
 
+	/*
+	 * Clear the special abort context for next time.
+	 */
+	if (TransactionAbortContext != NULL)
+		MemoryContextResetAndDeleteChildren(TransactionAbortContext);
+
 	/*
 	 * Delete the subxact local memory contexts. Its CurTransactionContext can
 	 * go too (note this also kills CurTransactionContexts from any children
@@ -1849,6 +1877,10 @@ AbortTransaction(void)
 	/* Prevent cancel/die interrupt while cleaning up */
 	HOLD_INTERRUPTS();
 
+	/* Make sure we have a valid memory context and resource owner */
+	AtAbort_Memory();
+	AtAbort_ResourceOwner();
+
 	/*
 	 * Release any LW locks we might be holding as quickly as possible.
 	 * (Regular locks, however, must be held till we finish aborting.)
@@ -1881,10 +1913,6 @@ AbortTransaction(void)
 	 */
 	s->state = TRANS_ABORT;
 
-	/* Make sure we have a valid memory context and resource owner */
-	AtAbort_Memory();
-	AtAbort_ResourceOwner();
-
 	/*
 	 * Reset user id which might have been changed transiently.  We cannot use
 	 * s->currentUser, since it may not be set yet; instead rely on internal
@@ -3704,15 +3732,12 @@ AbortSubTransaction(void)
 {
 	TransactionState s = CurrentTransactionState;
 
-	ShowTransactionState("AbortSubTransaction");
-
-	if (s->state != TRANS_INPROGRESS)
-		elog(WARNING, "AbortSubTransaction while in %s state",
-			 TransStateAsString(s->state));
-
+	/* Prevent cancel/die interrupt while cleaning up */
 	HOLD_INTERRUPTS();
 
-	s->state = TRANS_ABORT;
+	/* Make sure we have a valid memory context and resource owner */
+	AtSubAbort_Memory();
+	AtSubAbort_ResourceOwner();
 
 	/*
 	 * Release any LW locks we might be holding as quickly as possible.
@@ -3731,10 +3756,15 @@ AbortSubTransaction(void)
 	LockWaitCancel();
 
 	/*
-	 * do abort processing
+	 * check the current transaction state
 	 */
-	AtSubAbort_Memory();
-	AtSubAbort_ResourceOwner();
+	ShowTransactionState("AbortSubTransaction");
+
+	if (s->state != TRANS_INPROGRESS)
+		elog(WARNING, "AbortSubTransaction while in %s state",
+			 TransStateAsString(s->state));
+
+	s->state = TRANS_ABORT;
 
 	/*
 	 * We can skip all this stuff if the subxact failed before creating a

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.209 2006/10/04 00:29:51 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.210 2006/11/23 01:14:59 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1801,8 +1801,8 @@ ltrmark:;
  * during the current transaction tree.  (BEFORE triggers are fired
  * immediately so we don't need any persistent state about them.)  The struct
  * and most of its subsidiary data are kept in TopTransactionContext; however
- * the individual event records are kept in CurTransactionContext, so that
- * they will easily go away during subtransaction abort.
+ * the individual event records are kept in separate contexts, to make them
+ * easy to delete during subtransaction abort.
  *
  * Because the list of pending events can grow large, we go to some effort
  * to minimize memory consumption.	We do not use the generic List mechanism
@@ -1889,7 +1889,10 @@ typedef struct AfterTriggerEventList
  * events is the current list of deferred events.  This is global across
  * all subtransactions of the current transaction.	In a subtransaction
  * abort, we know that the events added by the subtransaction are at the
- * end of the list, so it is relatively easy to discard them.
+ * end of the list, so it is relatively easy to discard them.  The event
+ * structs themselves are stored in event_cxt if generated by the top-level
+ * transaction, else in per-subtransaction contexts identified by the
+ * entries in cxt_stack.
  *
  * query_depth is the current depth of nested AfterTriggerBeginQuery calls
  * (-1 when the stack is empty).
@@ -1931,6 +1934,8 @@ typedef struct AfterTriggersData
 	int			query_depth;	/* current query list index */
 	AfterTriggerEventList *query_stack; /* events pending from each query */
 	int			maxquerydepth;	/* allocated len of above array */
+	MemoryContext event_cxt;	/* top transaction's event context, if any */
+	MemoryContext *cxt_stack;	/* per-subtransaction event contexts */
 
 	/* these fields are just for resetting at subtrans abort: */
 
@@ -2464,7 +2469,11 @@ AfterTriggerBeginXact(void)
 						   8 * sizeof(AfterTriggerEventList));
 	afterTriggers->maxquerydepth = 8;
 
+	/* Context for events is created only when needed */
+	afterTriggers->event_cxt = NULL;
+
 	/* Subtransaction stack is empty until/unless needed */
+	afterTriggers->cxt_stack = NULL;
 	afterTriggers->state_stack = NULL;
 	afterTriggers->events_stack = NULL;
 	afterTriggers->depth_stack = NULL;
@@ -2626,8 +2635,18 @@ AfterTriggerEndXact(bool isCommit)
 	 * Forget everything we know about AFTER triggers.
 	 *
 	 * Since all the info is in TopTransactionContext or children thereof, we
-	 * need do nothing special to reclaim memory.
+	 * don't really need to do anything to reclaim memory.  However, the
+	 * pending-events list could be large, and so it's useful to discard
+	 * it as soon as possible --- especially if we are aborting because we
+	 * ran out of memory for the list!
+	 *
+	 * (Note: any event_cxts of child subtransactions could also be
+	 * deleted here, but we have no convenient way to find them, so we
+	 * leave it to TopTransactionContext reset to clean them up.)
 	 */
+	if (afterTriggers && afterTriggers->event_cxt)
+		MemoryContextDelete(afterTriggers->event_cxt);
+
 	afterTriggers = NULL;
 }
 
@@ -2649,7 +2668,10 @@ AfterTriggerBeginSubXact(void)
 		return;
 
 	/*
-	 * Allocate more space in the stacks if needed.
+	 * Allocate more space in the stacks if needed.  (Note: because the
+	 * minimum nest level of a subtransaction is 2, we waste the first
+	 * couple entries of each array; not worth the notational effort to
+	 * avoid it.)
 	 */
 	while (my_level >= afterTriggers->maxtransdepth)
 	{
@@ -2660,6 +2682,8 @@ AfterTriggerBeginSubXact(void)
 			old_cxt = MemoryContextSwitchTo(TopTransactionContext);
 
 #define DEFTRIG_INITALLOC 8
+			afterTriggers->cxt_stack = (MemoryContext *)
+				palloc(DEFTRIG_INITALLOC * sizeof(MemoryContext));
 			afterTriggers->state_stack = (SetConstraintState *)
 				palloc(DEFTRIG_INITALLOC * sizeof(SetConstraintState));
 			afterTriggers->events_stack = (AfterTriggerEventList *)
@@ -2677,6 +2701,9 @@ AfterTriggerBeginSubXact(void)
 			/* repalloc will keep the stacks in the same context */
 			int			new_alloc = afterTriggers->maxtransdepth * 2;
 
+			afterTriggers->cxt_stack = (MemoryContext *)
+				repalloc(afterTriggers->cxt_stack,
+						 new_alloc * sizeof(MemoryContext));
 			afterTriggers->state_stack = (SetConstraintState *)
 				repalloc(afterTriggers->state_stack,
 						 new_alloc * sizeof(SetConstraintState));
@@ -2695,8 +2722,10 @@ AfterTriggerBeginSubXact(void)
 
 	/*
 	 * Push the current information into the stack.  The SET CONSTRAINTS state
-	 * is not saved until/unless changed.
+	 * is not saved until/unless changed.  Likewise, we don't make a
+	 * per-subtransaction event context until needed.
 	 */
+	afterTriggers->cxt_stack[my_level] = NULL;
 	afterTriggers->state_stack[my_level] = NULL;
 	afterTriggers->events_stack[my_level] = afterTriggers->events;
 	afterTriggers->depth_stack[my_level] = afterTriggers->query_depth;
@@ -2742,7 +2771,23 @@ AfterTriggerEndSubXact(bool isCommit)
 	else
 	{
 		/*
-		 * Aborting --- restore the pointers from the stacks.
+		 * Aborting.  We don't really need to release the subxact's event_cxt,
+		 * since it will go away anyway when CurTransactionContext gets reset,
+		 * but doing so early in subxact abort helps free space we might need.
+		 *
+		 * (Note: any event_cxts of child subtransactions could also be
+		 * deleted here, but we have no convenient way to find them, so we
+		 * leave it to CurTransactionContext reset to clean them up.)
+		 */
+		if (afterTriggers->cxt_stack[my_level])
+		{
+			MemoryContextDelete(afterTriggers->cxt_stack[my_level]);
+			/* avoid double delete if repeated aborts */
+			afterTriggers->cxt_stack[my_level] = NULL;
+		}
+
+		/*
+		 * Restore the pointers from the stacks.
 		 */
 		afterTriggers->events = afterTriggers->events_stack[my_level];
 		afterTriggers->query_depth = afterTriggers->depth_stack[my_level];
@@ -2753,11 +2798,6 @@ AfterTriggerEndSubXact(bool isCommit)
 		if (afterTriggers->events.tail != NULL)
 			afterTriggers->events.tail->ate_next = NULL;
 
-		/*
-		 * We don't need to free the subtransaction's items, since the
-		 * CurTransactionContext will be reset shortly.
-		 */
-
 		/*
 		 * Restore the trigger state.  If the saved state is NULL, then this
 		 * subxact didn't save it, so it doesn't need restoring.
@@ -3204,6 +3244,8 @@ AfterTriggerSaveEvent(ResultRelInfo *relinfo, int event, bool row_trigger,
 {
 	Relation	rel = relinfo->ri_RelationDesc;
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
+	int			my_level = GetCurrentTransactionNestLevel();
+	MemoryContext *cxtptr;
 	AfterTriggerEvent new_event;
 	int			i;
 	int			ntriggers;
@@ -3294,12 +3336,29 @@ AfterTriggerSaveEvent(ResultRelInfo *relinfo, int event, bool row_trigger,
 		}
 
 		/*
-		 * Create a new event.	We use the CurTransactionContext so the event
-		 * will automatically go away if the subtransaction aborts.
+		 * If we don't yet have an event context for the current (sub)xact,
+		 * create one.  Make it a child of CurTransactionContext to ensure it
+		 * will go away if the subtransaction aborts.
+		 */
+		if (my_level > 1)			/* subtransaction? */
+		{
+			Assert(my_level < afterTriggers->maxtransdepth);
+			cxtptr = &afterTriggers->cxt_stack[my_level];
+		}
+		else
+			cxtptr = &afterTriggers->event_cxt;
+		if (*cxtptr == NULL)
+			*cxtptr = AllocSetContextCreate(CurTransactionContext,
+											"AfterTriggerEvents",
+											ALLOCSET_DEFAULT_MINSIZE,
+											ALLOCSET_DEFAULT_INITSIZE,
+											ALLOCSET_DEFAULT_MAXSIZE);
+
+		/*
+		 * Create a new event.
 		 */
 		new_event = (AfterTriggerEvent)
-			MemoryContextAlloc(CurTransactionContext,
-							   sizeof(AfterTriggerEventData));
+			MemoryContextAlloc(*cxtptr, sizeof(AfterTriggerEventData));
 		new_event->ate_next = NULL;
 		new_event->ate_event =
 			(event & TRIGGER_EVENT_OPMASK) |

src/backend/utils/mmgr/portalmem.c

@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.96 2006/10/04 00:30:04 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.97 2006/11/23 01:14:59 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -586,7 +586,7 @@ AtCommit_Portals(void)
  * Abort processing for portals.
  *
  * At this point we reset "active" status and run the cleanup hook if
- * present, but we can't release memory until the cleanup call.
+ * present, but we can't release the portal's memory until the cleanup call.
  *
  * The reason we need to reset active is so that we can replace the unnamed
  * portal, else we'll fail to execute ROLLBACK when it arrives.
@@ -625,6 +625,14 @@ AtAbort_Portals(void)
 		 * PortalDrop.
 		 */
 		portal->resowner = NULL;
+
+		/*
+		 * Although we can't delete the portal data structure proper, we can
+		 * release any memory in subsidiary contexts, such as executor state.
+		 * The cleanup hook was the last thing that might have needed data
+		 * there.
+		 */
+		MemoryContextDeleteChildren(PortalGetHeapMemory(portal));
 	}
 }
 
@@ -756,6 +764,14 @@ AtSubAbort_Portals(SubTransactionId mySubid,
 			 * run PortalDrop.
 			 */
 			portal->resowner = NULL;
+
+			/*
+			 * Although we can't delete the portal data structure proper, we
+			 * can release any memory in subsidiary contexts, such as executor
+			 * state.  The cleanup hook was the last thing that might have
+			 * needed data there.
+			 */
+			MemoryContextDeleteChildren(PortalGetHeapMemory(portal));
 		}
 	}
 }