Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit88e6ad3

Browse files
committed
Fix two memory leaks around force-storing tuples in slots.
As reported by Tom, when ExecStoreMinimalTuple() had to perform aconversion to store the minimal tuple in the slot, it forgot torespect the shouldFree flag, and leaked the tuple into the currentmemory context if true. Fix that by freeing the tuple in that case.Looking at the relevant code made me (Andres) realize that not havingthe shouldFree parameter to ExecForceStoreHeapTuple() was a badidea. Some callers had to locally implement the necessary logic, andin one case it was missing, creating a potential per-group leak innon-hashed aggregation.The choice to not free the tuple in ExecComputeStoredGenerated() isnot pretty, but not introduced by this commit - I'll start a separatediscussion about it.Reported-By: Tom LaneDiscussion:https://postgr.es/m/366.1555382816@sss.pgh.pa.us
1 parent4d5840c commit88e6ad3

File tree

7 files changed

+41
-20
lines changed

7 files changed

+41
-20
lines changed

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3711,9 +3711,7 @@ store_returning_result(PgFdwModifyState *fmstate,
37113711
* The returning slot will not necessarily be suitable to store
37123712
* heaptuples directly, so allow for conversion.
37133713
*/
3714-
ExecForceStoreHeapTuple(newtup,slot);
3715-
ExecMaterializeSlot(slot);
3716-
pfree(newtup);
3714+
ExecForceStoreHeapTuple(newtup,slot, true);
37173715
}
37183716
PG_CATCH();
37193717
{

‎src/backend/commands/trigger.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2586,7 +2586,7 @@ ExecBRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
25862586
}
25872587
elseif (newtuple!=oldtuple)
25882588
{
2589-
ExecForceStoreHeapTuple(newtuple,slot);
2589+
ExecForceStoreHeapTuple(newtuple,slot, false);
25902590

25912591
if (should_free)
25922592
heap_freetuple(oldtuple);
@@ -2668,7 +2668,7 @@ ExecIRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
26682668
}
26692669
elseif (newtuple!=oldtuple)
26702670
{
2671-
ExecForceStoreHeapTuple(newtuple,slot);
2671+
ExecForceStoreHeapTuple(newtuple,slot, false);
26722672

26732673
if (should_free)
26742674
heap_freetuple(oldtuple);
@@ -2797,7 +2797,7 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
27972797
else
27982798
{
27992799
trigtuple=fdw_trigtuple;
2800-
ExecForceStoreHeapTuple(trigtuple,slot);
2800+
ExecForceStoreHeapTuple(trigtuple,slot, false);
28012801
}
28022802

28032803
LocTriggerData.type=T_TriggerData;
@@ -2869,7 +2869,7 @@ ExecARDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
28692869
slot,
28702870
NULL);
28712871
else
2872-
ExecForceStoreHeapTuple(fdw_trigtuple,slot);
2872+
ExecForceStoreHeapTuple(fdw_trigtuple,slot, false);
28732873

28742874
AfterTriggerSaveEvent(estate,relinfo,TRIGGER_EVENT_DELETE,
28752875
true,slot,NULL,NIL,NULL,
@@ -2898,7 +2898,7 @@ ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
28982898
LocTriggerData.tg_oldtable=NULL;
28992899
LocTriggerData.tg_newtable=NULL;
29002900

2901-
ExecForceStoreHeapTuple(trigtuple,slot);
2901+
ExecForceStoreHeapTuple(trigtuple,slot, false);
29022902

29032903
for (i=0;i<trigdesc->numtriggers;i++)
29042904
{
@@ -3057,7 +3057,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
30573057
}
30583058
else
30593059
{
3060-
ExecForceStoreHeapTuple(fdw_trigtuple,oldslot);
3060+
ExecForceStoreHeapTuple(fdw_trigtuple,oldslot, false);
30613061
trigtuple=fdw_trigtuple;
30623062
}
30633063

@@ -3107,7 +3107,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
31073107
}
31083108
elseif (newtuple!=oldtuple)
31093109
{
3110-
ExecForceStoreHeapTuple(newtuple,newslot);
3110+
ExecForceStoreHeapTuple(newtuple,newslot, false);
31113111

31123112
/*
31133113
* If the tuple returned by the trigger / being stored, is the old
@@ -3164,7 +3164,7 @@ ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
31643164
oldslot,
31653165
NULL);
31663166
elseif (fdw_trigtuple!=NULL)
3167-
ExecForceStoreHeapTuple(fdw_trigtuple,oldslot);
3167+
ExecForceStoreHeapTuple(fdw_trigtuple,oldslot, false);
31683168

31693169
AfterTriggerSaveEvent(estate,relinfo,TRIGGER_EVENT_UPDATE,
31703170
true,oldslot,newslot,recheckIndexes,
@@ -3192,7 +3192,7 @@ ExecIRUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
31923192
LocTriggerData.tg_oldtable=NULL;
31933193
LocTriggerData.tg_newtable=NULL;
31943194

3195-
ExecForceStoreHeapTuple(trigtuple,oldslot);
3195+
ExecForceStoreHeapTuple(trigtuple,oldslot, false);
31963196

31973197
for (i=0;i<trigdesc->numtriggers;i++)
31983198
{
@@ -3228,7 +3228,7 @@ ExecIRUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
32283228
}
32293229
elseif (newtuple!=oldtuple)
32303230
{
3231-
ExecForceStoreHeapTuple(newtuple,newslot);
3231+
ExecForceStoreHeapTuple(newtuple,newslot, false);
32323232

32333233
if (should_free)
32343234
heap_freetuple(oldtuple);

‎src/backend/executor/execTuples.c

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,11 +1430,12 @@ ExecStoreMinimalTuple(MinimalTuple mtup,
14301430
*/
14311431
void
14321432
ExecForceStoreHeapTuple(HeapTupletuple,
1433-
TupleTableSlot*slot)
1433+
TupleTableSlot*slot,
1434+
boolshouldFree)
14341435
{
14351436
if (TTS_IS_HEAPTUPLE(slot))
14361437
{
1437-
ExecStoreHeapTuple(tuple,slot,false);
1438+
ExecStoreHeapTuple(tuple,slot,shouldFree);
14381439
}
14391440
elseif (TTS_IS_BUFFERTUPLE(slot))
14401441
{
@@ -1447,13 +1448,22 @@ ExecForceStoreHeapTuple(HeapTuple tuple,
14471448
oldContext=MemoryContextSwitchTo(slot->tts_mcxt);
14481449
bslot->base.tuple=heap_copytuple(tuple);
14491450
MemoryContextSwitchTo(oldContext);
1451+
1452+
if (shouldFree)
1453+
pfree(tuple);
14501454
}
14511455
else
14521456
{
14531457
ExecClearTuple(slot);
14541458
heap_deform_tuple(tuple,slot->tts_tupleDescriptor,
14551459
slot->tts_values,slot->tts_isnull);
14561460
ExecStoreVirtualTuple(slot);
1461+
1462+
if (shouldFree)
1463+
{
1464+
ExecMaterializeSlot(slot);
1465+
pfree(tuple);
1466+
}
14571467
}
14581468
}
14591469

@@ -1481,6 +1491,12 @@ ExecForceStoreMinimalTuple(MinimalTuple mtup,
14811491
heap_deform_tuple(&htup,slot->tts_tupleDescriptor,
14821492
slot->tts_values,slot->tts_isnull);
14831493
ExecStoreVirtualTuple(slot);
1494+
1495+
if (shouldFree)
1496+
{
1497+
ExecMaterializeSlot(slot);
1498+
pfree(mtup);
1499+
}
14841500
}
14851501
}
14861502

‎src/backend/executor/nodeAgg.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1806,7 +1806,7 @@ agg_retrieve_direct(AggState *aggstate)
18061806
* cleared from the slot.
18071807
*/
18081808
ExecForceStoreHeapTuple(aggstate->grp_firstTuple,
1809-
firstSlot);
1809+
firstSlot, true);
18101810
aggstate->grp_firstTuple=NULL;/* don't keep two pointers */
18111811

18121812
/* set up for first advance_aggregates call */

‎src/backend/executor/nodeIndexonlyscan.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ IndexOnlyNext(IndexOnlyScanState *node)
205205
*/
206206
Assert(slot->tts_tupleDescriptor->natts==
207207
scandesc->xs_hitupdesc->natts);
208-
ExecForceStoreHeapTuple(scandesc->xs_hitup,slot);
208+
ExecForceStoreHeapTuple(scandesc->xs_hitup,slot, false);
209209
}
210210
elseif (scandesc->xs_itup)
211211
StoreIndexTuple(slot,scandesc->xs_itup,scandesc->xs_itupdesc);

‎src/backend/executor/nodeModifyTable.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,12 @@ ExecComputeStoredGenerated(EState *estate, TupleTableSlot *slot)
317317

318318
oldtuple=ExecFetchSlotHeapTuple(slot, true,&should_free);
319319
newtuple=heap_modify_tuple(oldtuple,tupdesc,values,nulls,replaces);
320-
ExecForceStoreHeapTuple(newtuple,slot);
320+
/*
321+
* The tuple will be freed by way of the memory context - the slot might
322+
* only be cleared after the context is reset, and we'd thus potentially
323+
* double free.
324+
*/
325+
ExecForceStoreHeapTuple(newtuple,slot, false);
321326
if (should_free)
322327
heap_freetuple(oldtuple);
323328

@@ -979,7 +984,7 @@ ldelete:;
979984
slot=ExecGetReturningSlot(estate,resultRelInfo);
980985
if (oldtuple!=NULL)
981986
{
982-
ExecForceStoreHeapTuple(oldtuple,slot);
987+
ExecForceStoreHeapTuple(oldtuple,slot, false);
983988
}
984989
else
985990
{

‎src/include/executor/tuptable.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,9 @@ extern void ExecSetSlotDescriptor(TupleTableSlot *slot, TupleDesc tupdesc);
306306
externTupleTableSlot*ExecStoreHeapTuple(HeapTupletuple,
307307
TupleTableSlot*slot,
308308
boolshouldFree);
309-
externvoidExecForceStoreHeapTuple(HeapTupletuple,TupleTableSlot*slot);
309+
externvoidExecForceStoreHeapTuple(HeapTupletuple,
310+
TupleTableSlot*slot,
311+
boolshouldFree);
310312
externTupleTableSlot*ExecStoreBufferHeapTuple(HeapTupletuple,
311313
TupleTableSlot*slot,
312314
Bufferbuffer);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp