]> granicus.if.org Git - postgresql/commitdiff
Fix memory leaks in record_out() and record_send().
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 13 Nov 2012 19:44:35 +0000 (14:44 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 13 Nov 2012 19:45:43 +0000 (14:45 -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 ba7fde43132cde28c6db5df53101d12b9560632e..bfb3ccebbdcdb35907e09debd21ea7c37e1bf0b3 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);