From 273986bf0d39e5166eb15ba42ebff4803e23a688 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 13 Nov 2012 14:44:28 -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 13e574d4e8..d4ed7d0ca0 100644 --- a/src/backend/utils/adt/rowtypes.c +++ b/src/backend/utils/adt/rowtypes.c @@ -32,6 +32,7 @@ typedef struct ColumnIOData Oid column_type; Oid typiofunc; Oid typioparam; + bool typisvarlena; FmgrInfo proc; } ColumnIOData; @@ -364,6 +365,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; @@ -387,17 +389,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 */ @@ -416,17 +425,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, ')'); @@ -714,6 +729,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 */ @@ -734,23 +750,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); -- 2.40.0