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

Commitb006f4d

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 parent84a05d4 commitb006f4d

File tree

4 files changed

+68
-137
lines changed

4 files changed

+68
-137
lines changed

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

Lines changed: 55 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include"tcop/pquery.h"
2222
#include"utils/lsyscache.h"
2323
#include"utils/memdebug.h"
24+
#include"utils/memutils.h"
2425

2526

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

6668
/* ----------------
@@ -87,6 +89,7 @@ printtup_create_DR(CommandDest dest)
8789
self->attrinfo=NULL;
8890
self->nattrs=0;
8991
self->myinfo=NULL;
92+
self->tmpcontext=NULL;
9093

9194
return (DestReceiver*)self;
9295
}
@@ -124,6 +127,18 @@ printtup_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
124127
DR_printtup*myState= (DR_printtup*)self;
125128
Portalportal=myState->portal;
126129

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

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

@@ -313,8 +332,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
313332
for (i=0;i<natts;++i)
314333
{
315334
PrinttupAttrInfo*thisState=myState->myinfo+i;
316-
Datumorigattr=slot->tts_values[i],
317-
attr;
335+
Datumattr=slot->tts_values[i];
318336

319337
if (slot->tts_isnull[i])
320338
{
@@ -323,30 +341,15 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
323341
}
324342

325343
/*
326-
* If we have a toasted datum, forcibly detoast it here to avoid
327-
* memory leakage inside the type's output routine.
328-
*
329-
* Here we catch undefined bytes in tuples that are returned to the
344+
* Here we catch undefined bytes in datums that are returned to the
330345
* client without hitting disk; see comments at the related check in
331-
* PageAddItem(). Whether to test before or after detoast is somewhat
332-
* arbitrary, as is whether to test external/compressed data at all.
333-
* Undefined bytes in the pre-toast datum will have triggered Valgrind
334-
* errors in the compressor or toaster; any error detected here for
335-
* such datums would indicate an (unlikely) bug in a type-independent
336-
* facility. Therefore, this test is most useful for uncompressed,
337-
* non-external datums.
338-
*
339-
* We don't presently bother checking non-varlena datums for undefined
340-
* data. PageAddItem() does check them.
346+
* PageAddItem(). This test is most useful for uncompressed,
347+
* non-external datums, but we're quite likely to see such here when
348+
* testing new C functions.
341349
*/
342350
if (thisState->typisvarlena)
343-
{
344-
VALGRIND_CHECK_MEM_IS_DEFINED(origattr,VARSIZE_ANY(origattr));
345-
346-
attr=PointerGetDatum(PG_DETOAST_DATUM(origattr));
347-
}
348-
else
349-
attr=origattr;
351+
VALGRIND_CHECK_MEM_IS_DEFINED(DatumGetPointer(attr),
352+
VARSIZE_ANY(attr));
350353

351354
if (thisState->format==0)
352355
{
@@ -355,7 +358,6 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
355358

356359
outputstr=OutputFunctionCall(&thisState->finfo,attr);
357360
pq_sendcountedtext(&buf,outputstr,strlen(outputstr), false);
358-
pfree(outputstr);
359361
}
360362
else
361363
{
@@ -366,15 +368,14 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
366368
pq_sendint(&buf,VARSIZE(outputbytes)-VARHDRSZ,4);
367369
pq_sendbytes(&buf,VARDATA(outputbytes),
368370
VARSIZE(outputbytes)-VARHDRSZ);
369-
pfree(outputbytes);
370371
}
371-
372-
/* Clean up detoasted copy, if any */
373-
if (DatumGetPointer(attr)!=DatumGetPointer(origattr))
374-
pfree(DatumGetPointer(attr));
375372
}
376373

377374
pq_endmessage(&buf);
375+
376+
/* Return to caller's context, and flush row's temporary memory */
377+
MemoryContextSwitchTo(oldcontext);
378+
MemoryContextReset(myState->tmpcontext);
378379
}
379380

380381
/* ----------------
@@ -386,6 +387,7 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self)
386387
{
387388
TupleDesctypeinfo=slot->tts_tupleDescriptor;
388389
DR_printtup*myState= (DR_printtup*)self;
390+
MemoryContextoldcontext;
389391
StringInfoDatabuf;
390392
intnatts=typeinfo->natts;
391393
inti,
@@ -399,6 +401,9 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self)
399401
/* Make sure the tuple is fully deconstructed */
400402
slot_getallattrs(slot);
401403

404+
/* Switch into per-row context so we can recover memory below */
405+
oldcontext=MemoryContextSwitchTo(myState->tmpcontext);
406+
402407
/*
403408
* tell the frontend to expect new tuple data (in ASCII style)
404409
*/
@@ -430,34 +435,23 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self)
430435
for (i=0;i<natts;++i)
431436
{
432437
PrinttupAttrInfo*thisState=myState->myinfo+i;
433-
Datumorigattr=slot->tts_values[i],
434-
attr;
438+
Datumattr=slot->tts_values[i];
435439
char*outputstr;
436440

437441
if (slot->tts_isnull[i])
438442
continue;
439443

440444
Assert(thisState->format==0);
441445

442-
/*
443-
* If we have a toasted datum, forcibly detoast it here to avoid
444-
* memory leakage inside the type's output routine.
445-
*/
446-
if (thisState->typisvarlena)
447-
attr=PointerGetDatum(PG_DETOAST_DATUM(origattr));
448-
else
449-
attr=origattr;
450-
451446
outputstr=OutputFunctionCall(&thisState->finfo,attr);
452447
pq_sendcountedtext(&buf,outputstr,strlen(outputstr), true);
453-
pfree(outputstr);
454-
455-
/* Clean up detoasted copy, if any */
456-
if (DatumGetPointer(attr)!=DatumGetPointer(origattr))
457-
pfree(DatumGetPointer(attr));
458448
}
459449

460450
pq_endmessage(&buf);
451+
452+
/* Return to caller's context, and flush row's temporary memory */
453+
MemoryContextSwitchTo(oldcontext);
454+
MemoryContextReset(myState->tmpcontext);
461455
}
462456

463457
/* ----------------
@@ -474,6 +468,10 @@ printtup_shutdown(DestReceiver *self)
474468
myState->myinfo=NULL;
475469

476470
myState->attrinfo=NULL;
471+
472+
if (myState->tmpcontext)
473+
MemoryContextDelete(myState->tmpcontext);
474+
myState->tmpcontext=NULL;
477475
}
478476

479477
/* ----------------
@@ -536,39 +534,23 @@ debugtup(TupleTableSlot *slot, DestReceiver *self)
536534
TupleDesctypeinfo=slot->tts_tupleDescriptor;
537535
intnatts=typeinfo->natts;
538536
inti;
539-
Datumorigattr,
540-
attr;
537+
Datumattr;
541538
char*value;
542539
boolisnull;
543540
Oidtypoutput;
544541
booltypisvarlena;
545542

546543
for (i=0;i<natts;++i)
547544
{
548-
origattr=slot_getattr(slot,i+1,&isnull);
545+
attr=slot_getattr(slot,i+1,&isnull);
549546
if (isnull)
550547
continue;
551548
getTypeOutputInfo(typeinfo->attrs[i]->atttypid,
552549
&typoutput,&typisvarlena);
553550

554-
/*
555-
* If we have a toasted datum, forcibly detoast it here to avoid
556-
* memory leakage inside the type's output routine.
557-
*/
558-
if (typisvarlena)
559-
attr=PointerGetDatum(PG_DETOAST_DATUM(origattr));
560-
else
561-
attr=origattr;
562-
563551
value=OidOutputFunctionCall(typoutput,attr);
564552

565553
printatt((unsigned)i+1,typeinfo->attrs[i],value);
566-
567-
pfree(value);
568-
569-
/* Clean up detoasted copy, if any */
570-
if (DatumGetPointer(attr)!=DatumGetPointer(origattr))
571-
pfree(DatumGetPointer(attr));
572554
}
573555
printf("\t----\n");
574556
}
@@ -587,6 +569,7 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self)
587569
{
588570
TupleDesctypeinfo=slot->tts_tupleDescriptor;
589571
DR_printtup*myState= (DR_printtup*)self;
572+
MemoryContextoldcontext;
590573
StringInfoDatabuf;
591574
intnatts=typeinfo->natts;
592575
inti,
@@ -600,6 +583,9 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self)
600583
/* Make sure the tuple is fully deconstructed */
601584
slot_getallattrs(slot);
602585

586+
/* Switch into per-row context so we can recover memory below */
587+
oldcontext=MemoryContextSwitchTo(myState->tmpcontext);
588+
603589
/*
604590
* tell the frontend to expect new tuple data (in binary style)
605591
*/
@@ -631,35 +617,23 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self)
631617
for (i=0;i<natts;++i)
632618
{
633619
PrinttupAttrInfo*thisState=myState->myinfo+i;
634-
Datumorigattr=slot->tts_values[i],
635-
attr;
620+
Datumattr=slot->tts_values[i];
636621
bytea*outputbytes;
637622

638623
if (slot->tts_isnull[i])
639624
continue;
640625

641626
Assert(thisState->format==1);
642627

643-
/*
644-
* If we have a toasted datum, forcibly detoast it here to avoid
645-
* memory leakage inside the type's output routine.
646-
*/
647-
if (thisState->typisvarlena)
648-
attr=PointerGetDatum(PG_DETOAST_DATUM(origattr));
649-
else
650-
attr=origattr;
651-
652628
outputbytes=SendFunctionCall(&thisState->finfo,attr);
653-
/* We assume the result will not have been toasted */
654629
pq_sendint(&buf,VARSIZE(outputbytes)-VARHDRSZ,4);
655630
pq_sendbytes(&buf,VARDATA(outputbytes),
656631
VARSIZE(outputbytes)-VARHDRSZ);
657-
pfree(outputbytes);
658-
659-
/* Clean up detoasted copy, if any */
660-
if (DatumGetPointer(attr)!=DatumGetPointer(origattr))
661-
pfree(DatumGetPointer(attr));
662632
}
663633

664634
pq_endmessage(&buf);
635+
636+
/* Return to caller's context, and flush row's temporary memory */
637+
MemoryContextSwitchTo(oldcontext);
638+
MemoryContextReset(myState->tmpcontext);
665639
}

‎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