Prevent memory leaks from accumulating across printtup() calls.

Historically, printtup() has assumed that it could prevent memory leakage
by pfree'ing the string result of each output function and manually
managing detoasting of toasted values.  This amounts to assuming that
datatype output functions never leak any memory internally; an assumption
we've already decided to be bogus elsewhere, for example in COPY OUT.
range_out in particular is known to leak multiple kilobytes per call, as
noted in bug #8573 from Godfried Vanluffelen.  While we could go in and fix
that leak, it wouldn't be very notationally convenient, and in any case
there have been and undoubtedly will again be other leaks in other output
functions.  So what seems like the best solution is to run the output
functions in a temporary memory context that can be reset after each row,
as we're doing in COPY OUT.  Some quick experimentation suggests this is
actually 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 worrying
about query-lifespan leaks in standalone mode, and fixing that case
would be a bit tedious since debugtup() doesn't currently have any
startup or shutdown functions.

While at it, remove manual detoast management from several other
output-function call sites that had copied it from printtup().  This
doesn't make a lot of difference right now, but in view of recent
discussions about supporting "non-flattened" Datums, we're going to
want that code gone eventually anyway.

Back-patch to 9.2 where range_out was introduced.  We might eventually
decide to back-patch this further, but in the absence of known major
leaks in older output functions, I'll refrain for now.

diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c
index 4616106afe81e41150d613358c9dd8246fbbfe43..65c51abc8ff50100338aa1894f5a1c58f38b0a3e 100644 (file)
--- a/src/backend/access/common/printtup.c
+++ b/src/backend/access/common/printtup.c
@@ -20,6 +20,7 @@
 #include "libpq/pqformat.h"
 #include "tcop/pquery.h"
 #include "utils/lsyscache.h"
+#include "utils/memutils.h"
 
 
 static void printtup_startup(DestReceiver *self, int operation,
@@ -60,6 +61,7 @@ typedef struct
        TupleDesc       attrinfo;               /* The attr info we are set up for */
        int                     nattrs;
        PrinttupAttrInfo *myinfo;       /* Cached info about each attr */
+       MemoryContext tmpcontext;       /* Memory context for per-row workspace */
 } DR_printtup;
 
 /* ----------------
@@ -86,6 +88,7 @@ printtup_create_DR(CommandDest dest)
        self->attrinfo = NULL;
        self->nattrs = 0;
        self->myinfo = NULL;
+       self->tmpcontext = NULL;
 
        return (DestReceiver *) self;
 }
@@ -123,6 +126,18 @@ printtup_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
        DR_printtup *myState = (DR_printtup *) self;
        Portal          portal = myState->portal;
 
+       /*
+        * Create a temporary memory context that we can reset once per row to
+        * recover palloc'd memory.  This avoids any problems with leaks inside
+        * datatype output routines, and should be faster than retail pfree's
+        * anyway.
+        */
+       myState->tmpcontext = AllocSetContextCreate(CurrentMemoryContext,
+                                                                                               "printtup",
+                                                                                               ALLOCSET_DEFAULT_MINSIZE,
+                                                                                               ALLOCSET_DEFAULT_INITSIZE,
+                                                                                               ALLOCSET_DEFAULT_MAXSIZE);
+
        if (PG_PROTOCOL_MAJOR(FrontendProtocol) < 3)
        {
                /*
@@ -288,6 +303,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
 {
        TupleDesc       typeinfo = slot->tts_tupleDescriptor;
        DR_printtup *myState = (DR_printtup *) self;
+       MemoryContext oldcontext;
        StringInfoData buf;
        int                     natts = typeinfo->natts;
        int                     i;
@@ -299,8 +315,11 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
        /* Make sure the tuple is fully deconstructed */
        slot_getallattrs(slot);
 
+       /* Switch into per-row context so we can recover memory below */
+       oldcontext = MemoryContextSwitchTo(myState->tmpcontext);
+
        /*
-        * Prepare a DataRow message
+        * Prepare a DataRow message (note buffer is in per-row context)
         */
        pq_beginmessage(&buf, 'D');
 
@@ -312,8 +331,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
        for (i = 0; i < natts; ++i)
        {
                PrinttupAttrInfo *thisState = myState->myinfo + i;
-               Datum           origattr = slot->tts_values[i],
-                                       attr;
+               Datum           attr = slot->tts_values[i];
 
                if (slot->tts_isnull[i])
                {
@@ -321,15 +339,6 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
                        continue;
                }
 
-               /*
-                * If we have a toasted datum, forcibly detoast it here to avoid
-                * memory leakage inside the type's output routine.
-                */
-               if (thisState->typisvarlena)
-                       attr = PointerGetDatum(PG_DETOAST_DATUM(origattr));
-               else
-                       attr = origattr;
-
                if (thisState->format == 0)
                {
                        /* Text output */
@@ -337,7 +346,6 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
 
                        outputstr = OutputFunctionCall(&thisState->finfo, attr);
                        pq_sendcountedtext(&buf, outputstr, strlen(outputstr), false);
-                       pfree(outputstr);
                }
                else
                {
@@ -348,15 +356,14 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
                        pq_sendint(&buf, VARSIZE(outputbytes) - VARHDRSZ, 4);
                        pq_sendbytes(&buf, VARDATA(outputbytes),
                                                 VARSIZE(outputbytes) - VARHDRSZ);
-                       pfree(outputbytes);
                }
-
-               /* Clean up detoasted copy, if any */
-               if (DatumGetPointer(attr) != DatumGetPointer(origattr))
-                       pfree(DatumGetPointer(attr));
        }
 
        pq_endmessage(&buf);
+
+       /* Return to caller's context, and flush row's temporary memory */
+       MemoryContextSwitchTo(oldcontext);
+       MemoryContextReset(myState->tmpcontext);
 }
 
 /* ----------------
@@ -368,6 +375,7 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self)
 {
        TupleDesc       typeinfo = slot->tts_tupleDescriptor;
        DR_printtup *myState = (DR_printtup *) self;
+       MemoryContext oldcontext;
        StringInfoData buf;
        int                     natts = typeinfo->natts;
        int                     i,
@@ -381,6 +389,9 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self)
        /* Make sure the tuple is fully deconstructed */
        slot_getallattrs(slot);
 
+       /* Switch into per-row context so we can recover memory below */
+       oldcontext = MemoryContextSwitchTo(myState->tmpcontext);
+
        /*
         * tell the frontend to expect new tuple data (in ASCII style)
         */
@@ -412,8 +423,7 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self)
        for (i = 0; i < natts; ++i)
        {
                PrinttupAttrInfo *thisState = myState->myinfo + i;
-               Datum           origattr = slot->tts_values[i],
-                                       attr;
+               Datum           attr = slot->tts_values[i];
                char       *outputstr;
 
                if (slot->tts_isnull[i])
@@ -421,25 +431,15 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self)
 
                Assert(thisState->format == 0);
 
-               /*
-                * If we have a toasted datum, forcibly detoast it here to avoid
-                * memory leakage inside the type's output routine.
-                */
-               if (thisState->typisvarlena)
-                       attr = PointerGetDatum(PG_DETOAST_DATUM(origattr));
-               else
-                       attr = origattr;
-
                outputstr = OutputFunctionCall(&thisState->finfo, attr);
                pq_sendcountedtext(&buf, outputstr, strlen(outputstr), true);
-               pfree(outputstr);
-
-               /* Clean up detoasted copy, if any */
-               if (DatumGetPointer(attr) != DatumGetPointer(origattr))
-                       pfree(DatumGetPointer(attr));
        }
 
        pq_endmessage(&buf);
+
+       /* Return to caller's context, and flush row's temporary memory */
+       MemoryContextSwitchTo(oldcontext);
+       MemoryContextReset(myState->tmpcontext);
 }
 
 /* ----------------
@@ -456,6 +456,10 @@ printtup_shutdown(DestReceiver *self)
        myState->myinfo = NULL;
 
        myState->attrinfo = NULL;
+
+       if (myState->tmpcontext)
+               MemoryContextDelete(myState->tmpcontext);
+       myState->tmpcontext = NULL;
 }
 
 /* ----------------
@@ -518,8 +522,7 @@ debugtup(TupleTableSlot *slot, DestReceiver *self)
        TupleDesc       typeinfo = slot->tts_tupleDescriptor;
        int                     natts = typeinfo->natts;
        int                     i;
-       Datum           origattr,
-                               attr;
+       Datum           attr;
        char       *value;
        bool            isnull;
        Oid                     typoutput;
@@ -527,30 +530,15 @@ debugtup(TupleTableSlot *slot, DestReceiver *self)
 
        for (i = 0; i < natts; ++i)
        {
-               origattr = slot_getattr(slot, i + 1, &isnull);
+               attr = slot_getattr(slot, i + 1, &isnull);
                if (isnull)
                        continue;
                getTypeOutputInfo(typeinfo->attrs[i]->atttypid,
                                                  &typoutput, &typisvarlena);
 
-               /*
-                * If we have a toasted datum, forcibly detoast it here to avoid
-                * memory leakage inside the type's output routine.
-                */
-               if (typisvarlena)
-                       attr = PointerGetDatum(PG_DETOAST_DATUM(origattr));
-               else
-                       attr = origattr;
-
                value = OidOutputFunctionCall(typoutput, attr);
 
                printatt((unsigned) i + 1, typeinfo->attrs[i], value);
-
-               pfree(value);
-
-               /* Clean up detoasted copy, if any */
-               if (DatumGetPointer(attr) != DatumGetPointer(origattr))
-                       pfree(DatumGetPointer(attr));
        }
        printf("\t----\n");
 }
@@ -569,6 +557,7 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self)
 {
        TupleDesc       typeinfo = slot->tts_tupleDescriptor;
        DR_printtup *myState = (DR_printtup *) self;
+       MemoryContext oldcontext;
        StringInfoData buf;
        int                     natts = typeinfo->natts;
        int                     i,
@@ -582,6 +571,9 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self)
        /* Make sure the tuple is fully deconstructed */
        slot_getallattrs(slot);
 
+       /* Switch into per-row context so we can recover memory below */
+       oldcontext = MemoryContextSwitchTo(myState->tmpcontext);
+
        /*
         * tell the frontend to expect new tuple data (in binary style)
         */
@@ -613,8 +605,7 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self)
        for (i = 0; i < natts; ++i)
        {
                PrinttupAttrInfo *thisState = myState->myinfo + i;
-               Datum           origattr = slot->tts_values[i],
-                                       attr;
+               Datum           attr = slot->tts_values[i];
                bytea      *outputbytes;
 
                if (slot->tts_isnull[i])
@@ -622,26 +613,15 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self)
 
                Assert(thisState->format == 1);
 
-               /*
-                * If we have a toasted datum, forcibly detoast it here to avoid
-                * memory leakage inside the type's output routine.
-                */
-               if (thisState->typisvarlena)
-                       attr = PointerGetDatum(PG_DETOAST_DATUM(origattr));
-               else
-                       attr = origattr;
-
                outputbytes = SendFunctionCall(&thisState->finfo, attr);
-               /* We assume the result will not have been toasted */
                pq_sendint(&buf, VARSIZE(outputbytes) - VARHDRSZ, 4);
                pq_sendbytes(&buf, VARDATA(outputbytes),
                                         VARSIZE(outputbytes) - VARHDRSZ);
-               pfree(outputbytes);
-
-               /* Clean up detoasted copy, if any */
-               if (DatumGetPointer(attr) != DatumGetPointer(origattr))
-                       pfree(DatumGetPointer(attr));
        }
 
        pq_endmessage(&buf);
+
+       /* Return to caller's context, and flush row's temporary memory */
+       MemoryContextSwitchTo(oldcontext);
+       MemoryContextReset(myState->tmpcontext);
 }

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index b7428829f372330227eda53c858f47c467a45f35..b2b385e0cabe1f39616cbfd1930b3eb6e7bef6ae 100644 (file)
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -824,7 +824,6 @@ InsertOneValue(char *value, int i)
        Oid                     typioparam;
        Oid                     typinput;
        Oid                     typoutput;
-       char       *prt;
 
        AssertArg(i >= 0 && i < MAXATTR);
 
@@ -838,9 +837,14 @@ InsertOneValue(char *value, int i)
                                                  &typinput, &typoutput);
 
        values[i] = OidInputFunctionCall(typinput, value, typioparam, -1);
-       prt = OidOutputFunctionCall(typoutput, values[i]);
-       elog(DEBUG4, "inserted -> %s", prt);
-       pfree(prt);
+
+       /*
+        * We use ereport not elog here so that parameters aren't evaluated unless
+        * the message is going to be printed, which generally it isn't
+        */
+       ereport(DEBUG4,
+                       (errmsg_internal("inserted -> %s",
+                                                        OidOutputFunctionCall(typoutput, values[i]))));
 }
 
 /* ----------------

diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 96465e161e1bf24618438eea35998410be66517d..eae39b48e59917fb27238617d340dff70fbde4d8 100644 (file)
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -843,9 +843,7 @@ SPI_fname(TupleDesc tupdesc, int fnumber)
 char *
 SPI_getvalue(HeapTuple tuple, TupleDesc tupdesc, int fnumber)
 {
-       char       *result;
-       Datum           origval,
-                               val;
+       Datum           val;
        bool            isnull;
        Oid                     typoid,
                                foutoid;
@@ -860,7 +858,7 @@ SPI_getvalue(HeapTuple tuple, TupleDesc tupdesc, int fnumber)
                return NULL;
        }
 
-       origval = heap_getattr(tuple, fnumber, tupdesc, &isnull);
+       val = heap_getattr(tuple, fnumber, tupdesc, &isnull);
        if (isnull)
                return NULL;
 
@@ -871,22 +869,7 @@ SPI_getvalue(HeapTuple tuple, TupleDesc tupdesc, int fnumber)
 
        getTypeOutputInfo(typoid, &foutoid, &typisvarlena);
 
-       /*
-        * If we have a toasted datum, forcibly detoast it here to avoid memory
-        * leakage inside the type's output routine.
-        */
-       if (typisvarlena)
-               val = PointerGetDatum(PG_DETOAST_DATUM(origval));
-       else
-               val = origval;
-
-       result = OidOutputFunctionCall(foutoid, val);
-
-       /* Clean up detoasted copy, if any */
-       if (val != origval)
-               pfree(DatumGetPointer(val));
-
-       return result;
+       return OidOutputFunctionCall(foutoid, val);
 }
 
 Datum

diff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c
index bfb3ccebbdcdb35907e09debd21ea7c37e1bf0b3..c3e61ec1f6ea0c7af70a4ab2c54e2870b2523037 100644 (file)
--- a/src/backend/utils/adt/rowtypes.c
+++ b/src/backend/utils/adt/rowtypes.c
@@ -396,15 +396,7 @@ record_out(PG_FUNCTION_ARGS)
                        column_info->column_type = column_type;
                }
 
-               /*
-                * If we have a toasted datum, forcibly detoast it here to avoid
-                * memory leakage inside the type's output routine.
-                */
-               if (column_info->typisvarlena)
-                       attr = PointerGetDatum(PG_DETOAST_DATUM(values[i]));
-               else
-                       attr = values[i];
-
+               attr = values[i];
                value = OutputFunctionCall(&column_info->proc, attr);
 
                /* Detect whether we need double quotes for this value */
@@ -435,12 +427,6 @@ record_out(PG_FUNCTION_ARGS)
                }
                if (nq)
                        appendStringInfoCharMacro(&buf, '"');
-
-               pfree(value);
-
-               /* Clean up detoasted copy, if any */
-               if (DatumGetPointer(attr) != DatumGetPointer(values[i]))
-                       pfree(DatumGetPointer(attr));
        }
 
        appendStringInfoChar(&buf, ')');
@@ -757,27 +743,11 @@ record_send(PG_FUNCTION_ARGS)
                        column_info->column_type = column_type;
                }
 
-               /*
-                * If we have a toasted datum, forcibly detoast it here to avoid
-                * memory leakage inside the type's output routine.
-                */
-               if (column_info->typisvarlena)
-                       attr = PointerGetDatum(PG_DETOAST_DATUM(values[i]));
-               else
-                       attr = values[i];
-
+               attr = values[i];
                outputbytes = SendFunctionCall(&column_info->proc, attr);
-
-               /* We assume the result will not have been toasted */
                pq_sendint(&buf, VARSIZE(outputbytes) - VARHDRSZ, 4);
                pq_sendbytes(&buf, VARDATA(outputbytes),
                                         VARSIZE(outputbytes) - VARHDRSZ);
-
-               pfree(outputbytes);
-
-               /* Clean up detoasted copy, if any */
-               if (DatumGetPointer(attr) != DatumGetPointer(values[i]))
-                       pfree(DatumGetPointer(attr));
        }
 
        pfree(values);