From bd1693e89e7e6c458d0563f6cc67a7c99a45251a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 23 Jun 2016 10:55:59 -0400 Subject: [PATCH] Fix small memory leak in partial-aggregate deserialization functions. A deserialize function's result is short-lived data during partial aggregation, since we're just going to pass it to the combine function and then it's of no use anymore. However, the built-in deserialize functions allocated their results in the aggregate state context, resulting in a query-lifespan memory leak. It's probably not possible for this to amount to anything much at present, since the number of leaked results would only be the number of worker processes. But it might become a problem in future. To fix, don't use the same convenience subroutine for setting up results that the aggregate transition functions use. David Rowley Report: <10050.1466637736@sss.pgh.pa.us> --- src/backend/utils/adt/numeric.c | 56 ++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index f0b3b87f4c..f49e113e5e 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -3176,6 +3176,22 @@ makeNumericAggState(FunctionCallInfo fcinfo, bool calcSumX2) return state; } +/* + * Like makeNumericAggState(), but allocate the state in the current memory + * context. + */ +static NumericAggState * +makeNumericAggStateCurrentContext(bool calcSumX2) +{ + NumericAggState *state; + + state = (NumericAggState *) palloc0(sizeof(NumericAggState)); + state->calcSumX2 = calcSumX2; + state->agg_context = CurrentMemoryContext; + + return state; +} + /* * Accumulate a new input value for numeric aggregate functions. */ @@ -3374,7 +3390,7 @@ numeric_combine(PG_FUNCTION_ARGS) { old_context = MemoryContextSwitchTo(agg_context); - state1 = makeNumericAggState(fcinfo, true); + state1 = makeNumericAggStateCurrentContext(true); state1->N = state2->N; state1->NaNcount = state2->NaNcount; state1->maxScale = state2->maxScale; @@ -3465,7 +3481,7 @@ numeric_avg_combine(PG_FUNCTION_ARGS) { old_context = MemoryContextSwitchTo(agg_context); - state1 = makeNumericAggState(fcinfo, false); + state1 = makeNumericAggStateCurrentContext(false); state1->N = state2->N; state1->NaNcount = state2->NaNcount; state1->maxScale = state2->maxScale; @@ -3579,12 +3595,12 @@ numeric_avg_deserialize(PG_FUNCTION_ARGS) /* * Copy the bytea into a StringInfo so that we can "receive" it using the - * standard pq API. + * standard recv-function infrastructure. */ initStringInfo(&buf); appendBinaryStringInfo(&buf, VARDATA(sstate), VARSIZE(sstate) - VARHDRSZ); - result = makeNumericAggState(fcinfo, false); + result = makeNumericAggStateCurrentContext(false); /* N */ result->N = pq_getmsgint64(&buf); @@ -3691,12 +3707,12 @@ numeric_deserialize(PG_FUNCTION_ARGS) /* * Copy the bytea into a StringInfo so that we can "receive" it using the - * standard pq API. + * standard recv-function infrastructure. */ initStringInfo(&buf); appendBinaryStringInfo(&buf, VARDATA(sstate), VARSIZE(sstate) - VARHDRSZ); - result = makeNumericAggState(fcinfo, false); + result = makeNumericAggStateCurrentContext(false); /* N */ result->N = pq_getmsgint64(&buf); @@ -3803,6 +3819,21 @@ makeInt128AggState(FunctionCallInfo fcinfo, bool calcSumX2) return state; } +/* + * Like makeInt128AggState(), but allocate the state in the current memory + * context. + */ +static Int128AggState * +makeInt128AggStateCurrentContext(bool calcSumX2) +{ + Int128AggState *state; + + state = (Int128AggState *) palloc0(sizeof(Int128AggState)); + state->calcSumX2 = calcSumX2; + + return state; +} + /* * Accumulate a new input value for 128-bit aggregate functions. */ @@ -3831,9 +3862,11 @@ do_int128_discard(Int128AggState *state, int128 newval) typedef Int128AggState PolyNumAggState; #define makePolyNumAggState makeInt128AggState +#define makePolyNumAggStateCurrentContext makeInt128AggStateCurrentContext #else typedef NumericAggState PolyNumAggState; #define makePolyNumAggState makeNumericAggState +#define makePolyNumAggStateCurrentContext makeNumericAggStateCurrentContext #endif Datum @@ -4072,12 +4105,12 @@ numeric_poly_deserialize(PG_FUNCTION_ARGS) /* * Copy the bytea into a StringInfo so that we can "receive" it using the - * standard pq API. + * standard recv-function infrastructure. */ initStringInfo(&buf); appendBinaryStringInfo(&buf, VARDATA(sstate), VARSIZE(sstate) - VARHDRSZ); - result = makePolyNumAggState(fcinfo, false); + result = makePolyNumAggStateCurrentContext(false); /* N */ result->N = pq_getmsgint64(&buf); @@ -4210,7 +4243,8 @@ int8_avg_combine(PG_FUNCTION_ARGS) /* * int8_avg_serialize - * Serialize PolyNumAggState into bytea using the standard pq API. + * Serialize PolyNumAggState into bytea using the standard + * recv-function infrastructure. */ Datum int8_avg_serialize(PG_FUNCTION_ARGS) @@ -4284,12 +4318,12 @@ int8_avg_deserialize(PG_FUNCTION_ARGS) /* * Copy the bytea into a StringInfo so that we can "receive" it using the - * standard pq API. + * standard recv-function infrastructure. */ initStringInfo(&buf); appendBinaryStringInfo(&buf, VARDATA(sstate), VARSIZE(sstate) - VARHDRSZ); - result = makePolyNumAggState(fcinfo, false); + result = makePolyNumAggStateCurrentContext(false); /* N */ result->N = pq_getmsgint64(&buf);