Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Fix memory leaks in record_out() and record_send().
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 13 Nov 2012 19:44:46 +0000 (14:44 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 13 Nov 2012 19:46:06 +0000 (14:46 -0500)
record_out() leaks memory: it fails to free the strings returned by the
per-column output functions, and also is careless about detoasted values.
This results in a query-lifespan memory leakage when returning composite
values to the client, because printtup() runs the output functions in the
query-lifespan memory context.  Fix it to handle these issues the same way
printtup() does.  Also fix a similar leakage in record_send().

(At some point we might want to try to run output functions in
shorter-lived memory contexts, so that we don't need a zero-leakage policy
for them.  But that would be a significantly more invasive patch, which
doesn't seem like material for back-patching.)

In passing, use appendStringInfoCharMacro instead of appendStringInfoChar
in the innermost data-copying loop of record_out, to try to shave a few
cycles from this function's runtime.

Per trouble report from Carlos Henrique Reimer.  Back-patch to all
supported versions.

src/backend/utils/adt/rowtypes.c

index 50a541912806c05e62622b5f991ec4bbccf13c51..25d6af8ebf768caf543d93e6e5e4b10448fe8ff7 100644 (file)
@@ -31,6 +31,7 @@ typedef struct ColumnIOData
    Oid         column_type;
    Oid         typiofunc;
    Oid         typioparam;
+   bool        typisvarlena;
    FmgrInfo    proc;
 } ColumnIOData;
 
@@ -363,6 +364,7 @@ record_out(PG_FUNCTION_ARGS)
    {
        ColumnIOData *column_info = &my_extra->columns[i];
        Oid         column_type = tupdesc->attrs[i]->atttypid;
+       Datum       attr;
        char       *value;
        char       *tmp;
        bool        nq;
@@ -386,17 +388,24 @@ record_out(PG_FUNCTION_ARGS)
         */
        if (column_info->column_type != column_type)
        {
-           bool        typIsVarlena;
-
            getTypeOutputInfo(column_type,
                              &column_info->typiofunc,
-                             &typIsVarlena);
+                             &column_info->typisvarlena);
            fmgr_info_cxt(column_info->typiofunc, &column_info->proc,
                          fcinfo->flinfo->fn_mcxt);
            column_info->column_type = column_type;
        }
 
-       value = OutputFunctionCall(&column_info->proc, values[i]);
+       /*
+        * 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];
+
+       value = OutputFunctionCall(&column_info->proc, attr);
 
        /* Detect whether we need double quotes for this value */
        nq = (value[0] == '\0');    /* force quotes for empty string */
@@ -415,17 +424,23 @@ record_out(PG_FUNCTION_ARGS)
 
        /* And emit the string */
        if (nq)
-           appendStringInfoChar(&buf, '"');
+           appendStringInfoCharMacro(&buf, '"');
        for (tmp = value; *tmp; tmp++)
        {
            char        ch = *tmp;
 
            if (ch == '"' || ch == '\\')
-               appendStringInfoChar(&buf, ch);
-           appendStringInfoChar(&buf, ch);
+               appendStringInfoCharMacro(&buf, ch);
+           appendStringInfoCharMacro(&buf, ch);
        }
        if (nq)
-           appendStringInfoChar(&buf, '"');
+           appendStringInfoCharMacro(&buf, '"');
+
+       pfree(value);
+
+       /* Clean up detoasted copy, if any */
+       if (DatumGetPointer(attr) != DatumGetPointer(values[i]))
+           pfree(DatumGetPointer(attr));
    }
 
    appendStringInfoChar(&buf, ')');
@@ -713,6 +728,7 @@ record_send(PG_FUNCTION_ARGS)
    {
        ColumnIOData *column_info = &my_extra->columns[i];
        Oid         column_type = tupdesc->attrs[i]->atttypid;
+       Datum       attr;
        bytea      *outputbytes;
 
        /* Ignore dropped columns in datatype */
@@ -733,23 +749,35 @@ record_send(PG_FUNCTION_ARGS)
         */
        if (column_info->column_type != column_type)
        {
-           bool        typIsVarlena;
-
            getTypeBinaryOutputInfo(column_type,
                                    &column_info->typiofunc,
-                                   &typIsVarlena);
+                                   &column_info->typisvarlena);
            fmgr_info_cxt(column_info->typiofunc, &column_info->proc,
                          fcinfo->flinfo->fn_mcxt);
            column_info->column_type = column_type;
        }
 
-       outputbytes = SendFunctionCall(&column_info->proc, values[i]);
+       /*
+        * 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];
+
+       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);