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.

1 files changed, 76 insertions, 17 deletions

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 1ed15614ce4..58d8cbabfd4 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.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];
@@ -2754,11 +2799,6 @@ AfterTriggerEndSubXact(bool isCommit)
 			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) |