From 797ea5219ca1ddb54c9f9a5e908a1b2935a628f6 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 13 Nov 2012 14:44:46 -0500 Subject: [PATCH] Fix memory leaks in record_out() and record_send(). 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 | 52 ++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c index 50a5419128..25d6af8ebf 100644 --- a/src/backend/utils/adt/rowtypes.c +++ b/src/backend/utils/adt/rowtypes.c @@ -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);