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

Commitda2c6a9

Browse files
committed
Prevent memory leaks from accumulating across printtup() calls.
Historically, printtup() has assumed that it could prevent memory leakageby pfree'ing the string result of each output function and manuallymanaging detoasting of toasted values. This amounts to assuming thatdatatype output functions never leak any memory internally; an assumptionwe've already decided to be bogus elsewhere, for example in COPY OUT.range_out in particular is known to leak multiple kilobytes per call, asnoted in bug #8573 from Godfried Vanluffelen. While we could go in and fixthat leak, it wouldn't be very notationally convenient, and in any casethere have been and undoubtedly will again be other leaks in other outputfunctions. So what seems like the best solution is to run the outputfunctions in a temporary memory context that can be reset after each row,as we're doing in COPY OUT. Some quick experimentation suggests this isactually a tad faster than the retail pfree's anyway.This patch fixes all the variants of printtup, except for debugtup()which is used in standalone mode. It doesn't seem worth worryingabout query-lifespan leaks in standalone mode, and fixing that casewould be a bit tedious since debugtup() doesn't currently have anystartup or shutdown functions.While at it, remove manual detoast management from several otheroutput-function call sites that had copied it from printtup(). Thisdoesn't make a lot of difference right now, but in view of recentdiscussions about supporting "non-flattened" Datums, we're going towant that code gone eventually anyway.Back-patch to 9.2 where range_out was introduced. We might eventuallydecide to back-patch this further, but in the absence of known majorleaks in older output functions, I'll refrain for now.
1 parentc142a1a commitda2c6a9

File tree

4 files changed

+62
-125
lines changed

4 files changed

+62
-125
lines changed

‎src/backend/access/common/printtup.c

Lines changed: 49 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include"libpq/pqformat.h"
2121
#include"tcop/pquery.h"
2222
#include"utils/lsyscache.h"
23+
#include"utils/memutils.h"
2324

2425

2526
staticvoidprinttup_startup(DestReceiver*self,intoperation,
@@ -60,6 +61,7 @@ typedef struct
6061
TupleDescattrinfo;/* The attr info we are set up for */
6162
intnattrs;
6263
PrinttupAttrInfo*myinfo;/* Cached info about each attr */
64+
MemoryContexttmpcontext;/* Memory context for per-row workspace */
6365
}DR_printtup;
6466

6567
/* ----------------
@@ -86,6 +88,7 @@ printtup_create_DR(CommandDest dest)
8688
self->attrinfo=NULL;
8789
self->nattrs=0;
8890
self->myinfo=NULL;
91+
self->tmpcontext=NULL;
8992

9093
return (DestReceiver*)self;
9194
}
@@ -123,6 +126,18 @@ printtup_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
123126
DR_printtup*myState= (DR_printtup*)self;
124127
Portalportal=myState->portal;
125128

129+
/*
130+
* Create a temporary memory context that we can reset once per row to
131+
* recover palloc'd memory. This avoids any problems with leaks inside
132+
* datatype output routines, and should be faster than retail pfree's
133+
* anyway.
134+
*/
135+
myState->tmpcontext=AllocSetContextCreate(CurrentMemoryContext,
136+
"printtup",
137+
ALLOCSET_DEFAULT_MINSIZE,
138+
ALLOCSET_DEFAULT_INITSIZE,
139+
ALLOCSET_DEFAULT_MAXSIZE);
140+
126141
if (PG_PROTOCOL_MAJOR(FrontendProtocol)<3)
127142
{
128143
/*
@@ -288,6 +303,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
288303
{
289304
TupleDesctypeinfo=slot->tts_tupleDescriptor;
290305
DR_printtup*myState= (DR_printtup*)self;
306+
MemoryContextoldcontext;
291307
StringInfoDatabuf;
292308
intnatts=typeinfo->natts;
293309
inti;
@@ -299,8 +315,11 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
299315
/* Make sure the tuple is fully deconstructed */
300316
slot_getallattrs(slot);
301317

318+
/* Switch into per-row context so we can recover memory below */
319+
oldcontext=MemoryContextSwitchTo(myState->tmpcontext);
320+
302321
/*
303-
* Prepare a DataRow message
322+
* Prepare a DataRow message (note buffer is in per-row context)
304323
*/
305324
pq_beginmessage(&buf,'D');
306325

@@ -312,32 +331,21 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
312331
for (i=0;i<natts;++i)
313332
{
314333
PrinttupAttrInfo*thisState=myState->myinfo+i;
315-
Datumorigattr=slot->tts_values[i],
316-
attr;
334+
Datumattr=slot->tts_values[i];
317335

318336
if (slot->tts_isnull[i])
319337
{
320338
pq_sendint(&buf,-1,4);
321339
continue;
322340
}
323341

324-
/*
325-
* If we have a toasted datum, forcibly detoast it here to avoid
326-
* memory leakage inside the type's output routine.
327-
*/
328-
if (thisState->typisvarlena)
329-
attr=PointerGetDatum(PG_DETOAST_DATUM(origattr));
330-
else
331-
attr=origattr;
332-
333342
if (thisState->format==0)
334343
{
335344
/* Text output */
336345
char*outputstr;
337346

338347
outputstr=OutputFunctionCall(&thisState->finfo,attr);
339348
pq_sendcountedtext(&buf,outputstr,strlen(outputstr), false);
340-
pfree(outputstr);
341349
}
342350
else
343351
{
@@ -348,15 +356,14 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
348356
pq_sendint(&buf,VARSIZE(outputbytes)-VARHDRSZ,4);
349357
pq_sendbytes(&buf,VARDATA(outputbytes),
350358
VARSIZE(outputbytes)-VARHDRSZ);
351-
pfree(outputbytes);
352359
}
353-
354-
/* Clean up detoasted copy, if any */
355-
if (DatumGetPointer(attr)!=DatumGetPointer(origattr))
356-
pfree(DatumGetPointer(attr));
357360
}
358361

359362
pq_endmessage(&buf);
363+
364+
/* Return to caller's context, and flush row's temporary memory */
365+
MemoryContextSwitchTo(oldcontext);
366+
MemoryContextReset(myState->tmpcontext);
360367
}
361368

362369
/* ----------------
@@ -368,6 +375,7 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self)
368375
{
369376
TupleDesctypeinfo=slot->tts_tupleDescriptor;
370377
DR_printtup*myState= (DR_printtup*)self;
378+
MemoryContextoldcontext;
371379
StringInfoDatabuf;
372380
intnatts=typeinfo->natts;
373381
inti,
@@ -381,6 +389,9 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self)
381389
/* Make sure the tuple is fully deconstructed */
382390
slot_getallattrs(slot);
383391

392+
/* Switch into per-row context so we can recover memory below */
393+
oldcontext=MemoryContextSwitchTo(myState->tmpcontext);
394+
384395
/*
385396
* tell the frontend to expect new tuple data (in ASCII style)
386397
*/
@@ -412,34 +423,23 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self)
412423
for (i=0;i<natts;++i)
413424
{
414425
PrinttupAttrInfo*thisState=myState->myinfo+i;
415-
Datumorigattr=slot->tts_values[i],
416-
attr;
426+
Datumattr=slot->tts_values[i];
417427
char*outputstr;
418428

419429
if (slot->tts_isnull[i])
420430
continue;
421431

422432
Assert(thisState->format==0);
423433

424-
/*
425-
* If we have a toasted datum, forcibly detoast it here to avoid
426-
* memory leakage inside the type's output routine.
427-
*/
428-
if (thisState->typisvarlena)
429-
attr=PointerGetDatum(PG_DETOAST_DATUM(origattr));
430-
else
431-
attr=origattr;
432-
433434
outputstr=OutputFunctionCall(&thisState->finfo,attr);
434435
pq_sendcountedtext(&buf,outputstr,strlen(outputstr), true);
435-
pfree(outputstr);
436-
437-
/* Clean up detoasted copy, if any */
438-
if (DatumGetPointer(attr)!=DatumGetPointer(origattr))
439-
pfree(DatumGetPointer(attr));
440436
}
441437

442438
pq_endmessage(&buf);
439+
440+
/* Return to caller's context, and flush row's temporary memory */
441+
MemoryContextSwitchTo(oldcontext);
442+
MemoryContextReset(myState->tmpcontext);
443443
}
444444

445445
/* ----------------
@@ -456,6 +456,10 @@ printtup_shutdown(DestReceiver *self)
456456
myState->myinfo=NULL;
457457

458458
myState->attrinfo=NULL;
459+
460+
if (myState->tmpcontext)
461+
MemoryContextDelete(myState->tmpcontext);
462+
myState->tmpcontext=NULL;
459463
}
460464

461465
/* ----------------
@@ -518,39 +522,23 @@ debugtup(TupleTableSlot *slot, DestReceiver *self)
518522
TupleDesctypeinfo=slot->tts_tupleDescriptor;
519523
intnatts=typeinfo->natts;
520524
inti;
521-
Datumorigattr,
522-
attr;
525+
Datumattr;
523526
char*value;
524527
boolisnull;
525528
Oidtypoutput;
526529
booltypisvarlena;
527530

528531
for (i=0;i<natts;++i)
529532
{
530-
origattr=slot_getattr(slot,i+1,&isnull);
533+
attr=slot_getattr(slot,i+1,&isnull);
531534
if (isnull)
532535
continue;
533536
getTypeOutputInfo(typeinfo->attrs[i]->atttypid,
534537
&typoutput,&typisvarlena);
535538

536-
/*
537-
* If we have a toasted datum, forcibly detoast it here to avoid
538-
* memory leakage inside the type's output routine.
539-
*/
540-
if (typisvarlena)
541-
attr=PointerGetDatum(PG_DETOAST_DATUM(origattr));
542-
else
543-
attr=origattr;
544-
545539
value=OidOutputFunctionCall(typoutput,attr);
546540

547541
printatt((unsigned)i+1,typeinfo->attrs[i],value);
548-
549-
pfree(value);
550-
551-
/* Clean up detoasted copy, if any */
552-
if (DatumGetPointer(attr)!=DatumGetPointer(origattr))
553-
pfree(DatumGetPointer(attr));
554542
}
555543
printf("\t----\n");
556544
}
@@ -569,6 +557,7 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self)
569557
{
570558
TupleDesctypeinfo=slot->tts_tupleDescriptor;
571559
DR_printtup*myState= (DR_printtup*)self;
560+
MemoryContextoldcontext;
572561
StringInfoDatabuf;
573562
intnatts=typeinfo->natts;
574563
inti,
@@ -582,6 +571,9 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self)
582571
/* Make sure the tuple is fully deconstructed */
583572
slot_getallattrs(slot);
584573

574+
/* Switch into per-row context so we can recover memory below */
575+
oldcontext=MemoryContextSwitchTo(myState->tmpcontext);
576+
585577
/*
586578
* tell the frontend to expect new tuple data (in binary style)
587579
*/
@@ -613,35 +605,23 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self)
613605
for (i=0;i<natts;++i)
614606
{
615607
PrinttupAttrInfo*thisState=myState->myinfo+i;
616-
Datumorigattr=slot->tts_values[i],
617-
attr;
608+
Datumattr=slot->tts_values[i];
618609
bytea*outputbytes;
619610

620611
if (slot->tts_isnull[i])
621612
continue;
622613

623614
Assert(thisState->format==1);
624615

625-
/*
626-
* If we have a toasted datum, forcibly detoast it here to avoid
627-
* memory leakage inside the type's output routine.
628-
*/
629-
if (thisState->typisvarlena)
630-
attr=PointerGetDatum(PG_DETOAST_DATUM(origattr));
631-
else
632-
attr=origattr;
633-
634616
outputbytes=SendFunctionCall(&thisState->finfo,attr);
635-
/* We assume the result will not have been toasted */
636617
pq_sendint(&buf,VARSIZE(outputbytes)-VARHDRSZ,4);
637618
pq_sendbytes(&buf,VARDATA(outputbytes),
638619
VARSIZE(outputbytes)-VARHDRSZ);
639-
pfree(outputbytes);
640-
641-
/* Clean up detoasted copy, if any */
642-
if (DatumGetPointer(attr)!=DatumGetPointer(origattr))
643-
pfree(DatumGetPointer(attr));
644620
}
645621

646622
pq_endmessage(&buf);
623+
624+
/* Return to caller's context, and flush row's temporary memory */
625+
MemoryContextSwitchTo(oldcontext);
626+
MemoryContextReset(myState->tmpcontext);
647627
}

‎src/backend/bootstrap/bootstrap.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -835,7 +835,6 @@ InsertOneValue(char *value, int i)
835835
Oidtypioparam;
836836
Oidtypinput;
837837
Oidtypoutput;
838-
char*prt;
839838

840839
AssertArg(i >=0&&i<MAXATTR);
841840

@@ -849,9 +848,14 @@ InsertOneValue(char *value, int i)
849848
&typinput,&typoutput);
850849

851850
values[i]=OidInputFunctionCall(typinput,value,typioparam,-1);
852-
prt=OidOutputFunctionCall(typoutput,values[i]);
853-
elog(DEBUG4,"inserted -> %s",prt);
854-
pfree(prt);
851+
852+
/*
853+
* We use ereport not elog here so that parameters aren't evaluated unless
854+
* the message is going to be printed, which generally it isn't
855+
*/
856+
ereport(DEBUG4,
857+
(errmsg_internal("inserted -> %s",
858+
OidOutputFunctionCall(typoutput,values[i]))));
855859
}
856860

857861
/* ----------------

‎src/backend/executor/spi.c

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -869,9 +869,7 @@ SPI_fname(TupleDesc tupdesc, int fnumber)
869869
char*
870870
SPI_getvalue(HeapTupletuple,TupleDesctupdesc,intfnumber)
871871
{
872-
char*result;
873-
Datumorigval,
874-
val;
872+
Datumval;
875873
boolisnull;
876874
Oidtypoid,
877875
foutoid;
@@ -886,7 +884,7 @@ SPI_getvalue(HeapTuple tuple, TupleDesc tupdesc, int fnumber)
886884
returnNULL;
887885
}
888886

889-
origval=heap_getattr(tuple,fnumber,tupdesc,&isnull);
887+
val=heap_getattr(tuple,fnumber,tupdesc,&isnull);
890888
if (isnull)
891889
returnNULL;
892890

@@ -897,22 +895,7 @@ SPI_getvalue(HeapTuple tuple, TupleDesc tupdesc, int fnumber)
897895

898896
getTypeOutputInfo(typoid,&foutoid,&typisvarlena);
899897

900-
/*
901-
* If we have a toasted datum, forcibly detoast it here to avoid memory
902-
* leakage inside the type's output routine.
903-
*/
904-
if (typisvarlena)
905-
val=PointerGetDatum(PG_DETOAST_DATUM(origval));
906-
else
907-
val=origval;
908-
909-
result=OidOutputFunctionCall(foutoid,val);
910-
911-
/* Clean up detoasted copy, if any */
912-
if (val!=origval)
913-
pfree(DatumGetPointer(val));
914-
915-
returnresult;
898+
returnOidOutputFunctionCall(foutoid,val);
916899
}
917900

918901
Datum

0 commit comments

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