Skip to content

Commit c9fe128

Browse files
tglsfdctglsfdc
committed
Clean up per-tuple memory leaks in trigger firing and plpgsql
expression evaluation.
1 parent 59a3a40 commit c9fe128

File tree

9 files changed

+160
-147
lines changed

9 files changed

+160
-147
lines changed

‎src/backend/commands/copy.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/commands/copy.c,v 1.130 2001/01/19 06:54:57 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/commands/copy.c,v 1.131 2001/01/22 00:50:06 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -705,6 +705,9 @@ CopyFrom(Relation rel, bool binary, bool oids, FILE *fp,
705705

706706
lineno++;
707707

708+
/* Reset the per-output-tuple exprcontext */
709+
ResetPerTupleExprContext(estate);
710+
708711
/* Initialize all values for row to NULL */
709712
MemSet(values, 0, attr_count * sizeof(Datum));
710713
MemSet(nulls, 'n', attr_count * sizeof(char));
@@ -861,7 +864,7 @@ CopyFrom(Relation rel, bool binary, bool oids, FILE *fp,
861864
{
862865
HeapTuple newtuple;
863866

864-
newtuple = ExecBRInsertTriggers(rel, tuple);
867+
newtuple = ExecBRInsertTriggers(estate, rel, tuple);
865868

866869
if (newtuple == NULL) /* "do nothing" */
867870
skip_tuple = true;
@@ -895,7 +898,7 @@ CopyFrom(Relation rel, bool binary, bool oids, FILE *fp,
895898
/* AFTER ROW INSERT Triggers */
896899
if (rel->trigdesc &&
897900
rel->trigdesc->n_after_row[TRIGGER_EVENT_INSERT] > 0)
898-
ExecARInsertTriggers(rel, tuple);
901+
ExecARInsertTriggers(estate, rel, tuple);
899902
}
900903

901904
for (i = 0; i < attr_count; i++)

‎src/backend/commands/trigger.c

Lines changed: 57 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.82 2000/12/18 00:44:46 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/commands/trigger.c,v 1.83 2001/01/22 00:50:07 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -36,7 +36,8 @@ static void DescribeTrigger(TriggerDesc *trigdesc, Trigger *trigger);
3636
static HeapTuple GetTupleForTrigger(EState *estate, ItemPointer tid,
3737
TupleTableSlot **newSlot);
3838
static HeapTuple ExecCallTriggerFunc(Trigger *trigger,
39-
TriggerData *trigdata);
39+
TriggerData *trigdata,
40+
MemoryContext per_tuple_context);
4041
static void DeferredTriggerSaveEvent(Relation rel, int event,
4142
HeapTuple oldtup, HeapTuple newtup);
4243

@@ -831,10 +832,13 @@ equalTriggerDescs(TriggerDesc *trigdesc1, TriggerDesc *trigdesc2)
831832
}
832833

833834
static HeapTuple
834-
ExecCallTriggerFunc(Trigger *trigger, TriggerData *trigdata)
835+
ExecCallTriggerFunc(Trigger *trigger,
836+
TriggerData *trigdata,
837+
MemoryContext per_tuple_context)
835838
{
836839
FunctionCallInfoData fcinfo;
837840
Datum result;
841+
MemoryContext oldContext;
838842

839843
/*
840844
* Fmgr lookup info is cached in the Trigger structure,
@@ -843,6 +847,14 @@ ExecCallTriggerFunc(Trigger *trigger, TriggerData *trigdata)
843847
if (trigger->tgfunc.fn_oid == InvalidOid)
844848
fmgr_info(trigger->tgfoid, &trigger->tgfunc);
845849

850+
/*
851+
* Do the function evaluation in the per-tuple memory context,
852+
* so that leaked memory will be reclaimed once per tuple.
853+
* Note in particular that any new tuple created by the trigger function
854+
* will live till the end of the tuple cycle.
855+
*/
856+
oldContext = MemoryContextSwitchTo(per_tuple_context);
857+
846858
/*
847859
* Call the function, passing no arguments but setting a context.
848860
*/
@@ -853,6 +865,8 @@ ExecCallTriggerFunc(Trigger *trigger, TriggerData *trigdata)
853865

854866
result = FunctionCallInvoke(&fcinfo);
855867

868+
MemoryContextSwitchTo(oldContext);
869+
856870
/*
857871
* Trigger protocol allows function to return a null pointer,
858872
* but NOT to set the isnull result flag.
@@ -865,7 +879,7 @@ ExecCallTriggerFunc(Trigger *trigger, TriggerData *trigdata)
865879
}
866880

867881
HeapTuple
868-
ExecBRInsertTriggers(Relation rel, HeapTuple trigtuple)
882+
ExecBRInsertTriggers(EState *estate, Relation rel, HeapTuple trigtuple)
869883
{
870884
int ntrigs = rel->trigdesc->n_before_row[TRIGGER_EVENT_INSERT];
871885
Trigger **trigger = rel->trigdesc->tg_before_row[TRIGGER_EVENT_INSERT];
@@ -884,20 +898,20 @@ ExecBRInsertTriggers(Relation rel, HeapTuple trigtuple)
884898
continue;
885899
LocTriggerData.tg_trigtuple = oldtuple = newtuple;
886900
LocTriggerData.tg_trigger = trigger[i];
887-
newtuple = ExecCallTriggerFunc(trigger[i], &LocTriggerData);
901+
newtuple = ExecCallTriggerFunc(trigger[i], &LocTriggerData,
902+
GetPerTupleMemoryContext(estate));
903+
if (oldtuple != newtuple && oldtuple != trigtuple)
904+
heap_freetuple(oldtuple);
888905
if (newtuple == NULL)
889906
break;
890-
else if (oldtuple != newtuple && oldtuple != trigtuple)
891-
heap_freetuple(oldtuple);
892907
}
893908
return newtuple;
894909
}
895910

896911
void
897-
ExecARInsertTriggers(Relation rel, HeapTuple trigtuple)
912+
ExecARInsertTriggers(EState *estate, Relation rel, HeapTuple trigtuple)
898913
{
899914
DeferredTriggerSaveEvent(rel, TRIGGER_EVENT_INSERT, NULL, trigtuple);
900-
return;
901915
}
902916

903917
bool
@@ -926,7 +940,8 @@ ExecBRDeleteTriggers(EState *estate, ItemPointer tupleid)
926940
continue;
927941
LocTriggerData.tg_trigtuple = trigtuple;
928942
LocTriggerData.tg_trigger = trigger[i];
929-
newtuple = ExecCallTriggerFunc(trigger[i], &LocTriggerData);
943+
newtuple = ExecCallTriggerFunc(trigger[i], &LocTriggerData,
944+
GetPerTupleMemoryContext(estate));
930945
if (newtuple == NULL)
931946
break;
932947
if (newtuple != trigtuple)
@@ -944,7 +959,7 @@ ExecARDeleteTriggers(EState *estate, ItemPointer tupleid)
944959
HeapTuple trigtuple = GetTupleForTrigger(estate, tupleid, NULL);
945960

946961
DeferredTriggerSaveEvent(rel, TRIGGER_EVENT_DELETE, trigtuple, NULL);
947-
return;
962+
heap_freetuple(trigtuple);
948963
}
949964

950965
HeapTuple
@@ -981,11 +996,12 @@ ExecBRUpdateTriggers(EState *estate, ItemPointer tupleid, HeapTuple newtuple)
981996
LocTriggerData.tg_trigtuple = trigtuple;
982997
LocTriggerData.tg_newtuple = oldtuple = newtuple;
983998
LocTriggerData.tg_trigger = trigger[i];
984-
newtuple = ExecCallTriggerFunc(trigger[i], &LocTriggerData);
999+
newtuple = ExecCallTriggerFunc(trigger[i], &LocTriggerData,
1000+
GetPerTupleMemoryContext(estate));
1001+
if (oldtuple != newtuple && oldtuple != intuple)
1002+
heap_freetuple(oldtuple);
9851003
if (newtuple == NULL)
9861004
break;
987-
else if (oldtuple != newtuple && oldtuple != intuple)
988-
heap_freetuple(oldtuple);
9891005
}
9901006
heap_freetuple(trigtuple);
9911007
return newtuple;
@@ -998,7 +1014,7 @@ ExecARUpdateTriggers(EState *estate, ItemPointer tupleid, HeapTuple newtuple)
9981014
HeapTuple trigtuple = GetTupleForTrigger(estate, tupleid, NULL);
9991015

10001016
DeferredTriggerSaveEvent(rel, TRIGGER_EVENT_UPDATE, trigtuple, newtuple);
1001-
return;
1017+
heap_freetuple(trigtuple);
10021018
}
10031019

10041020

@@ -1236,7 +1252,7 @@ deferredTriggerGetPreviousEvent(Oid relid, ItemPointer ctid)
12361252
}
12371253

12381254
elog(ERROR,
1239-
"deferredTriggerGetPreviousEvent(): event for tuple %s not found",
1255+
"deferredTriggerGetPreviousEvent: event for tuple %s not found",
12401256
DatumGetCString(DirectFunctionCall1(tidout, PointerGetDatum(ctid))));
12411257
return NULL;
12421258
}
@@ -1250,7 +1266,8 @@ deferredTriggerGetPreviousEvent(Oid relid, ItemPointer ctid)
12501266
* ----------
12511267
*/
12521268
static void
1253-
deferredTriggerExecute(DeferredTriggerEvent event, int itemno)
1269+
deferredTriggerExecute(DeferredTriggerEvent event, int itemno,
1270+
MemoryContext per_tuple_context)
12541271
{
12551272
Relation rel;
12561273
TriggerData LocTriggerData;
@@ -1271,15 +1288,15 @@ deferredTriggerExecute(DeferredTriggerEvent event, int itemno)
12711288
ItemPointerCopy(&(event->dte_oldctid), &(oldtuple.t_self));
12721289
heap_fetch(rel, SnapshotAny, &oldtuple, &oldbuffer);
12731290
if (!oldtuple.t_data)
1274-
elog(ERROR, "deferredTriggerExecute(): failed to fetch old tuple");
1291+
elog(ERROR, "deferredTriggerExecute: failed to fetch old tuple");
12751292
}
12761293

12771294
if (ItemPointerIsValid(&(event->dte_newctid)))
12781295
{
12791296
ItemPointerCopy(&(event->dte_newctid), &(newtuple.t_self));
12801297
heap_fetch(rel, SnapshotAny, &newtuple, &newbuffer);
12811298
if (!newtuple.t_data)
1282-
elog(ERROR, "deferredTriggerExecute(): failed to fetch new tuple");
1299+
elog(ERROR, "deferredTriggerExecute: failed to fetch new tuple");
12831300
}
12841301

12851302
/* ----------
@@ -1320,7 +1337,9 @@ deferredTriggerExecute(DeferredTriggerEvent event, int itemno)
13201337
* updated tuple.
13211338
* ----------
13221339
*/
1323-
rettuple = ExecCallTriggerFunc(LocTriggerData.tg_trigger, &LocTriggerData);
1340+
rettuple = ExecCallTriggerFunc(LocTriggerData.tg_trigger,
1341+
&LocTriggerData,
1342+
per_tuple_context);
13241343
if (rettuple != NULL && rettuple != &oldtuple && rettuple != &newtuple)
13251344
heap_freetuple(rettuple);
13261345

@@ -1359,6 +1378,7 @@ deferredTriggerInvokeEvents(bool immediate_only)
13591378
int still_deferred_ones;
13601379
int eventno = -1;
13611380
int i;
1381+
MemoryContext per_tuple_context;
13621382

13631383
/* ----------
13641384
* For now we process all events - to speedup transaction blocks
@@ -1369,10 +1389,21 @@ deferredTriggerInvokeEvents(bool immediate_only)
13691389
* SET CONSTRAINTS ... command finishes and calls EndQuery.
13701390
* ----------
13711391
*/
1392+
1393+
/* Make a per-tuple memory context for trigger function calls */
1394+
per_tuple_context =
1395+
AllocSetContextCreate(CurrentMemoryContext,
1396+
"DeferredTriggerTupleContext",
1397+
0,
1398+
ALLOCSET_DEFAULT_INITSIZE,
1399+
ALLOCSET_DEFAULT_MAXSIZE);
1400+
13721401
foreach(el, deftrig_events)
13731402
{
13741403
eventno++;
13751404

1405+
MemoryContextReset(per_tuple_context);
1406+
13761407
/* ----------
13771408
* Get the event and check if it is completely done.
13781409
* ----------
@@ -1409,7 +1440,7 @@ deferredTriggerInvokeEvents(bool immediate_only)
14091440
* So let's fire it...
14101441
* ----------
14111442
*/
1412-
deferredTriggerExecute(event, i);
1443+
deferredTriggerExecute(event, i, per_tuple_context);
14131444
event->dte_item[i].dti_state |= TRIGGER_DEFERRED_DONE;
14141445
}
14151446

@@ -1421,6 +1452,8 @@ deferredTriggerInvokeEvents(bool immediate_only)
14211452
if (!still_deferred_ones)
14221453
event->dte_event |= TRIGGER_DEFERRED_DONE;
14231454
}
1455+
1456+
MemoryContextDelete(per_tuple_context);
14241457
}
14251458

14261459

@@ -1866,13 +1899,10 @@ DeferredTriggerSaveEvent(Relation rel, int event,
18661899
* Check if we're interested in this row at all
18671900
* ----------
18681901
*/
1869-
if (rel->trigdesc->n_after_row[TRIGGER_EVENT_INSERT] == 0 &&
1870-
rel->trigdesc->n_after_row[TRIGGER_EVENT_UPDATE] == 0 &&
1871-
rel->trigdesc->n_after_row[TRIGGER_EVENT_DELETE] == 0 &&
1872-
rel->trigdesc->n_before_row[TRIGGER_EVENT_INSERT] == 0 &&
1873-
rel->trigdesc->n_before_row[TRIGGER_EVENT_UPDATE] == 0 &&
1874-
rel->trigdesc->n_before_row[TRIGGER_EVENT_DELETE] == 0)
1902+
ntriggers = rel->trigdesc->n_after_row[event];
1903+
if (ntriggers <= 0)
18751904
return;
1905+
triggers = rel->trigdesc->tg_after_row[event];
18761906

18771907
/* ----------
18781908
* Get the CTID's of OLD and NEW
@@ -1893,9 +1923,6 @@ DeferredTriggerSaveEvent(Relation rel, int event,
18931923
*/
18941924
oldcxt = MemoryContextSwitchTo(deftrig_cxt);
18951925

1896-
ntriggers = rel->trigdesc->n_after_row[event];
1897-
triggers = rel->trigdesc->tg_after_row[event];
1898-
18991926
new_size = sizeof(DeferredTriggerEventData) +
19001927
ntriggers * sizeof(DeferredTriggerEventItem);
19011928

0 commit comments

Comments
 (0)