mirror/postgresql
2
0
Fork 0
mirror of https://git.postgresql.org/git/postgresql.git synced 2025-03-01 19:45:33 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix an old bug in after-trigger handling: AfterTriggerEndQuery took the

Browse Source 
address of afterTriggers->query_stack[afterTriggers->query_depth] and hung
onto it through all its firings of triggers.  However, if a trigger causes
sufficiently many nested query executions, query_stack will get repalloc'd
bigger, leaving AfterTriggerEndQuery --- and hence afterTriggerInvokeEvents
--- using a stale pointer.

So far as I can find, the only consequence of this error is to stomp on a
couple of words of already-freed memory; which would lead to a failure only if
that chunk had already gotten re-allocated for something else.  So it's hard
to exhibit a simple failure case, but this is surely a bug.

I noticed this while working on my recent patch to reduce pending-trigger
space usage.  The present patch is mighty ugly, because it requires making
afterTriggerInvokeEvents know about all the possible event lists it might get
called on.  Fortunately, this is only needed in back branches because CVS HEAD
avoids the problem in a different way: afterTriggerInvokeEvents only touches
the passed AfterTriggerEventList pointer once at startup.  Back branches are
stable enough that wiring in knowledge of all possible call usages doesn't
seem like a killer problem.

Back-patch to 8.0.  7.4's trigger code is completely different and doesn't
seem to have the problem (it doesn't even use repalloc).
This commit is contained in:
Tom Lane 2008-10-25 03:32:44 +00:00
parent 13a7d7f3cc
commit d4a1e5b0a3
1 changed files with 19 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/backend/commands/trigger.c
View File

@ -7,7 +7,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.227 2008/01/02 23:34:42 tgl Exp $
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.227.2.1 2008/10/25 03:32:44 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -2596,7 +2596,9 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
* afterTriggerInvokeEvents()
*
* Scan the given event list for events that are marked as to be fired
* in the current firing cycle, and fire them.
* in the current firing cycle, and fire them. query_depth is the index in
* afterTriggers->query_stack, or -1 to examine afterTriggers->events.
* (We have to be careful here because query_stack could move under us.)
*
* If estate isn't NULL, we use its result relation info to avoid repeated
* openings and closing of trigger target relations. If it is NULL, we
@ -2608,11 +2610,12 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
* ----------
*/
static void
afterTriggerInvokeEvents(AfterTriggerEventList *events,
afterTriggerInvokeEvents(int query_depth,
CommandId firing_id,
EState *estate,
bool delete_ok)
{
AfterTriggerEventList *events;
AfterTriggerEvent event,
prev_event;
MemoryContext per_tuple_context;
@ -2638,6 +2641,7 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
ALLOCSET_DEFAULT_MAXSIZE);
prev_event = NULL;
events = (query_depth >= 0) ? &afterTriggers->query_stack[query_depth] : &afterTriggers->events;
event = events->head;
while (event != NULL)
@ -2699,7 +2703,10 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
if (prev_event)
prev_event->ate_next = next_event;
else
{
events = (query_depth >= 0) ? &afterTriggers->query_stack[query_depth] : &afterTriggers->events;
events->head = next_event;
}
pfree(event);
}
else
@ -2712,6 +2719,7 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
}
/* Update list tail pointer in case we just deleted tail event */
events = (query_depth >= 0) ? &afterTriggers->query_stack[query_depth] : &afterTriggers->events;
events->tail = prev_event;
/* Release working resources */
@ -2831,8 +2839,6 @@ AfterTriggerBeginQuery(void)
void
AfterTriggerEndQuery(EState *estate)
{
AfterTriggerEventList *events;
/* Must be inside a transaction */
Assert(afterTriggers != NULL);
@ -2849,18 +2855,20 @@ AfterTriggerEndQuery(EState *estate)
* IMMEDIATE: all events we have decided to defer will be available for it
* to fire.
*
* We loop in case a trigger queues more events.
* We loop in case a trigger queues more events at the same query level
* (is that even possible?). Be careful here: firing a trigger could
* result in query_stack being repalloc'd, so we can't save its address
* across afterTriggerInvokeEvents calls.
*
* If we find no firable events, we don't have to increment
* firing_counter.
*/
events = &afterTriggers->query_stack[afterTriggers->query_depth];
while (afterTriggerMarkEvents(events, &afterTriggers->events, true))
while (afterTriggerMarkEvents(&afterTriggers->query_stack[afterTriggers->query_depth], &afterTriggers->events, true))
{
CommandId firing_id = afterTriggers->firing_counter++;
/* OK to delete the immediate events after processing them */
afterTriggerInvokeEvents(events, firing_id, estate, true);
afterTriggerInvokeEvents(afterTriggers->query_depth, firing_id, estate, true);
}
afterTriggers->query_depth--;
@ -2906,7 +2914,7 @@ AfterTriggerFireDeferred(void)
{
CommandId firing_id = afterTriggers->firing_counter++;
afterTriggerInvokeEvents(events, firing_id, NULL, true);
afterTriggerInvokeEvents(-1, firing_id, NULL, true);
}
Assert(events->head == NULL);
@ -3471,7 +3479,7 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt)
* but we'd better not if inside a subtransaction, since the
* subtransaction could later get rolled back.
*/
afterTriggerInvokeEvents(events, firing_id, NULL,
afterTriggerInvokeEvents(-1, firing_id, NULL,
!IsSubTransaction());
}
}