rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix I/O-conversion-related memory leaks in plpgsql. 

Datatype I/O functions are allowed to leak memory in CurrentMemoryContext,
since they are generally called in short-lived contexts.  However, plpgsql
calls such functions for purposes of type conversion, and was calling them
in its procedure context.  Therefore, any leaked memory would not be
recovered until the end of the plpgsql function.  If such a conversion
was done within a loop, quite a bit of memory could get consumed.  Fix by
calling such functions in the transient "eval_econtext", and adjust other
logic to match.  Back-patch to all supported versions.

Andres Freund, Jan Urbański, Tom Lane
This commit is contained in:
Tom Lane 2012-02-11 18:06:24 -05:00
parent 59de132f9a
commit 58a9596ed4
1 changed files with 96 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

157
src/pl/plpgsql/src/pl_exec.c
View File

@ -188,14 +188,17 @@ static void exec_move_row(PLpgSQL_execstate *estate,
static HeapTuple make_tuple_from_row(PLpgSQL_execstate *estate, static HeapTuple make_tuple_from_row(PLpgSQL_execstate *estate,
PLpgSQL_row *row, PLpgSQL_row *row,
TupleDesc tupdesc); TupleDesc tupdesc);
static char *convert_value_to_string(Datum value, Oid valtype); static char *convert_value_to_string(PLpgSQL_execstate *estate,
static Datum exec_cast_value(Datum value, Oid valtype, Datum value, Oid valtype);
static Datum exec_cast_value(PLpgSQL_execstate *estate,
Datum value, Oid valtype,
Oid reqtype, Oid reqtype,
FmgrInfo *reqinput, FmgrInfo *reqinput,
Oid reqtypioparam, Oid reqtypioparam,
int32 reqtypmod, int32 reqtypmod,
bool isnull); bool isnull);
static Datum exec_simple_cast_value(Datum value, Oid valtype, static Datum exec_simple_cast_value(PLpgSQL_execstate *estate,
Datum value, Oid valtype,
Oid reqtype, int32 reqtypmod, Oid reqtype, int32 reqtypmod,
bool isnull); bool isnull);
static void exec_init_tuple_store(PLpgSQL_execstate *estate); static void exec_init_tuple_store(PLpgSQL_execstate *estate);
@ -295,6 +298,8 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo)
/* If arg is null, treat it as an empty row */ /* If arg is null, treat it as an empty row */
exec_move_row(&estate, NULL, row, NULL, NULL); exec_move_row(&estate, NULL, row, NULL, NULL);
} }
/* clean up after exec_move_row() */
exec_eval_cleanup(&estate);
} }
break; break;
@ -430,7 +435,9 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo)
else else
{ {
/* Cast value to proper type */ /* Cast value to proper type */
estate.retval = exec_cast_value(estate.retval, estate.rettype, estate.retval = exec_cast_value(&estate,
estate.retval,
estate.rettype,
func->fn_rettype, func->fn_rettype,
&(func->fn_retinput), &(func->fn_retinput),
func->fn_rettypioparam, func->fn_rettypioparam,
@ -1757,7 +1764,7 @@ exec_stmt_fori(PLpgSQL_execstate *estate, PLpgSQL_stmt_fori *stmt)
* Get the value of the lower bound * Get the value of the lower bound
*/ */
value = exec_eval_expr(estate, stmt->lower, &isnull, &valtype); value = exec_eval_expr(estate, stmt->lower, &isnull, &valtype);
value = exec_cast_value(value, valtype, var->datatype->typoid, value = exec_cast_value(estate, value, valtype, var->datatype->typoid,
&(var->datatype->typinput), &(var->datatype->typinput),
var->datatype->typioparam, var->datatype->typioparam,
var->datatype->atttypmod, isnull); var->datatype->atttypmod, isnull);
@ -1772,7 +1779,7 @@ exec_stmt_fori(PLpgSQL_execstate *estate, PLpgSQL_stmt_fori *stmt)
* Get the value of the upper bound * Get the value of the upper bound
*/ */
value = exec_eval_expr(estate, stmt->upper, &isnull, &valtype); value = exec_eval_expr(estate, stmt->upper, &isnull, &valtype);
value = exec_cast_value(value, valtype, var->datatype->typoid, value = exec_cast_value(estate, value, valtype, var->datatype->typoid,
&(var->datatype->typinput), &(var->datatype->typinput),
var->datatype->typioparam, var->datatype->typioparam,
var->datatype->atttypmod, isnull); var->datatype->atttypmod, isnull);
@ -1789,7 +1796,7 @@ exec_stmt_fori(PLpgSQL_execstate *estate, PLpgSQL_stmt_fori *stmt)
if (stmt->step) if (stmt->step)
{ {
value = exec_eval_expr(estate, stmt->step, &isnull, &valtype); value = exec_eval_expr(estate, stmt->step, &isnull, &valtype);
value = exec_cast_value(value, valtype, var->datatype->typoid, value = exec_cast_value(estate, value, valtype, var->datatype->typoid,
&(var->datatype->typinput), &(var->datatype->typinput),
var->datatype->typioparam, var->datatype->typioparam,
var->datatype->atttypmod, isnull); var->datatype->atttypmod, isnull);
@ -2440,7 +2447,8 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
errmsg("wrong result type supplied in RETURN NEXT"))); errmsg("wrong result type supplied in RETURN NEXT")));
/* coerce type if needed */ /* coerce type if needed */
retval = exec_simple_cast_value(retval, retval = exec_simple_cast_value(estate,
retval,
var->datatype->typoid, var->datatype->typoid,
tupdesc->attrs[0]->atttypid, tupdesc->attrs[0]->atttypid,
tupdesc->attrs[0]->atttypmod, tupdesc->attrs[0]->atttypmod,
@ -2511,7 +2519,8 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
&rettype); &rettype);
/* coerce type if needed */ /* coerce type if needed */
retval = exec_simple_cast_value(retval, retval = exec_simple_cast_value(estate,
retval,
rettype, rettype,
tupdesc->attrs[0]->atttypid, tupdesc->attrs[0]->atttypid,
tupdesc->attrs[0]->atttypmod, tupdesc->attrs[0]->atttypmod,
@ -2519,8 +2528,6 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
tuplestore_putvalues(estate->tuple_store, tupdesc, tuplestore_putvalues(estate->tuple_store, tupdesc,
&retval, &isNull); &retval, &isNull);
exec_eval_cleanup(estate);
} }
else else
{ {
@ -2537,6 +2544,8 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
heap_freetuple(tuple); heap_freetuple(tuple);
} }
exec_eval_cleanup(estate);
return PLPGSQL_RC_OK; return PLPGSQL_RC_OK;
} }
@ -2726,7 +2735,9 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
if (paramisnull) if (paramisnull)
extval = "<NULL>"; extval = "<NULL>";
else else
extval = convert_value_to_string(paramvalue, paramtypeid); extval = convert_value_to_string(estate,
paramvalue,
paramtypeid);
appendStringInfoString(&ds, extval); appendStringInfoString(&ds, extval);
current_param = lnext(current_param); current_param = lnext(current_param);
exec_eval_cleanup(estate); exec_eval_cleanup(estate);
@ -2764,7 +2775,7 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
errmsg("RAISE statement option cannot be null"))); errmsg("RAISE statement option cannot be null")));
extval = convert_value_to_string(optionvalue, optiontypeid); extval = convert_value_to_string(estate, optionvalue, optiontypeid);
switch (opt->opt_type) switch (opt->opt_type)
{ {
@ -3183,6 +3194,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
} }
/* Clean up */ /* Clean up */
exec_eval_cleanup(estate);
SPI_freetuptable(SPI_tuptable); SPI_freetuptable(SPI_tuptable);
} }
else else
@ -3228,7 +3240,10 @@ exec_stmt_dynexecute(PLpgSQL_execstate *estate,
errmsg("query string argument of EXECUTE is null"))); errmsg("query string argument of EXECUTE is null")));
/* Get the C-String representation */ /* Get the C-String representation */
querystr = convert_value_to_string(query, restype); querystr = convert_value_to_string(estate, query, restype);
/* copy it out of the temporary context before we clean up */
querystr = pstrdup(querystr);
exec_eval_cleanup(estate); exec_eval_cleanup(estate);
@ -3361,6 +3376,8 @@ exec_stmt_dynexecute(PLpgSQL_execstate *estate,
/* Put the first result row into the target */ /* Put the first result row into the target */
exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc); exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc);
} }
/* clean up after exec_move_row() */
exec_eval_cleanup(estate);
} }
else else
{ {
@ -3634,6 +3651,7 @@ exec_stmt_fetch(PLpgSQL_execstate *estate, PLpgSQL_stmt_fetch *stmt)
else else
exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc); exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc);
exec_eval_cleanup(estate);
SPI_freetuptable(tuptab); SPI_freetuptable(tuptab);
} }
else else
@ -3733,7 +3751,7 @@ exec_assign_c_string(PLpgSQL_execstate *estate, PLpgSQL_datum *target,
/* ---------- /* ----------
* exec_assign_value Put a value into a target field * exec_assign_value Put a value into a target field
* *
* Note: in some code paths, this may leak memory in the eval_econtext; * Note: in some code paths, this will leak memory in the eval_econtext;
* we assume that will be cleaned up later by exec_eval_cleanup. We cannot * we assume that will be cleaned up later by exec_eval_cleanup. We cannot
* call exec_eval_cleanup here for fear of destroying the input Datum value. * call exec_eval_cleanup here for fear of destroying the input Datum value.
* ---------- * ----------
@ -3753,7 +3771,10 @@ exec_assign_value(PLpgSQL_execstate *estate,
PLpgSQL_var *var = (PLpgSQL_var *) target; PLpgSQL_var *var = (PLpgSQL_var *) target;
Datum newvalue; Datum newvalue;
newvalue = exec_cast_value(value, valtype, var->datatype->typoid, newvalue = exec_cast_value(estate,
value,
valtype,
var->datatype->typoid,
&(var->datatype->typinput), &(var->datatype->typinput),
var->datatype->typioparam, var->datatype->typioparam,
var->datatype->atttypmod, var->datatype->atttypmod,
@ -3766,19 +3787,14 @@ exec_assign_value(PLpgSQL_execstate *estate,
var->refname))); var->refname)));
/* /*
* If type is by-reference, make sure we have a freshly * If type is by-reference, copy the new value (which is
* palloc'd copy; the originally passed value may not live as * probably in the eval_econtext) into the procedure's
* long as the variable! But we don't need to re-copy if * memory context.
* exec_cast_value performed a conversion; its output must
* already be palloc'd.
*/ */
if (!var->datatype->typbyval && !*isNull) if (!var->datatype->typbyval && !*isNull)
{
if (newvalue == value)
newvalue = datumCopy(newvalue, newvalue = datumCopy(newvalue,
false, false,
var->datatype->typlen); var->datatype->typlen);
}
/* /*
* Now free the old value. (We can't do this any earlier * Now free the old value. (We can't do this any earlier
@ -3894,7 +3910,6 @@ exec_assign_value(PLpgSQL_execstate *estate,
Datum *values; Datum *values;
bool *nulls; bool *nulls;
bool *replaces; bool *replaces;
void *mustfree;
bool attisnull; bool attisnull;
Oid atttype; Oid atttype;
int32 atttypmod; int32 atttypmod;
@ -3946,22 +3961,14 @@ exec_assign_value(PLpgSQL_execstate *estate,
atttype = SPI_gettypeid(rec->tupdesc, fno + 1); atttype = SPI_gettypeid(rec->tupdesc, fno + 1);
atttypmod = rec->tupdesc->attrs[fno]->atttypmod; atttypmod = rec->tupdesc->attrs[fno]->atttypmod;
attisnull = *isNull; attisnull = *isNull;
values[fno] = exec_simple_cast_value(value, values[fno] = exec_simple_cast_value(estate,
value,
valtype, valtype,
atttype, atttype,
atttypmod, atttypmod,
attisnull); attisnull);
nulls[fno] = attisnull; nulls[fno] = attisnull;
/*
* Avoid leaking the result of exec_simple_cast_value, if it
* performed a conversion to a pass-by-ref type.
*/
if (!attisnull && values[fno] != value && !get_typbyval(atttype))
mustfree = DatumGetPointer(values[fno]);
else
mustfree = NULL;
/* /*
* Now call heap_modify_tuple() to create a new tuple that * Now call heap_modify_tuple() to create a new tuple that
* replaces the old one in the record. * replaces the old one in the record.
@ -3978,8 +3985,6 @@ exec_assign_value(PLpgSQL_execstate *estate,
pfree(values); pfree(values);
pfree(nulls); pfree(nulls);
pfree(replaces); pfree(replaces);
if (mustfree)
pfree(mustfree);
break; break;
} }
@ -4002,6 +4007,7 @@ exec_assign_value(PLpgSQL_execstate *estate,
ArrayType *oldarrayval; ArrayType *oldarrayval;
ArrayType *newarrayval; ArrayType *newarrayval;
SPITupleTable *save_eval_tuptable; SPITupleTable *save_eval_tuptable;
MemoryContext oldcontext;
/* /*
* We need to do subscript evaluation, which might require * We need to do subscript evaluation, which might require
@ -4118,7 +4124,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
estate->eval_tuptable = save_eval_tuptable; estate->eval_tuptable = save_eval_tuptable;
/* Coerce source value to match array element type. */ /* Coerce source value to match array element type. */
coerced_value = exec_simple_cast_value(value, coerced_value = exec_simple_cast_value(estate,
value,
valtype, valtype,
arrayelem->elemtypoid, arrayelem->elemtypoid,
arrayelem->arraytypmod, arrayelem->arraytypmod,
@ -4138,6 +4145,9 @@ exec_assign_value(PLpgSQL_execstate *estate,
(oldarrayisnull || *isNull)) (oldarrayisnull || *isNull))
return; return;
/* oldarrayval and newarrayval should be short-lived */
oldcontext = MemoryContextSwitchTo(estate->eval_econtext->ecxt_per_tuple_memory);
if (oldarrayisnull) if (oldarrayisnull)
oldarrayval = construct_empty_array(arrayelem->elemtypoid); oldarrayval = construct_empty_array(arrayelem->elemtypoid);
else else
@ -4156,13 +4166,7 @@ exec_assign_value(PLpgSQL_execstate *estate,
arrayelem->elemtypbyval, arrayelem->elemtypbyval,
arrayelem->elemtypalign); arrayelem->elemtypalign);
/* MemoryContextSwitchTo(oldcontext);
* Avoid leaking the result of exec_simple_cast_value, if it
* performed a conversion to a pass-by-ref type.
*/
if (!*isNull && coerced_value != value &&
!arrayelem->elemtypbyval)
pfree(DatumGetPointer(coerced_value));
/* /*
* Assign the new array to the base variable. It's never NULL * Assign the new array to the base variable. It's never NULL
@ -4174,11 +4178,6 @@ exec_assign_value(PLpgSQL_execstate *estate,
exec_assign_value(estate, target, exec_assign_value(estate, target,
PointerGetDatum(newarrayval), PointerGetDatum(newarrayval),
arrayelem->arraytypoid, isNull); arrayelem->arraytypoid, isNull);
/*
* Avoid leaking the modified array value, too.
*/
pfree(newarrayval);
break; break;
} }
@ -4514,7 +4513,7 @@ exec_eval_integer(PLpgSQL_execstate *estate,
Oid exprtypeid; Oid exprtypeid;
exprdatum = exec_eval_expr(estate, expr, isNull, &exprtypeid); exprdatum = exec_eval_expr(estate, expr, isNull, &exprtypeid);
exprdatum = exec_simple_cast_value(exprdatum, exprtypeid, exprdatum = exec_simple_cast_value(estate, exprdatum, exprtypeid,
INT4OID, -1, INT4OID, -1,
*isNull); *isNull);
return DatumGetInt32(exprdatum); return DatumGetInt32(exprdatum);
@ -4536,7 +4535,7 @@ exec_eval_boolean(PLpgSQL_execstate *estate,
Oid exprtypeid; Oid exprtypeid;
exprdatum = exec_eval_expr(estate, expr, isNull, &exprtypeid); exprdatum = exec_eval_expr(estate, expr, isNull, &exprtypeid);
exprdatum = exec_simple_cast_value(exprdatum, exprtypeid, exprdatum = exec_simple_cast_value(estate, exprdatum, exprtypeid,
BOOLOID, -1, BOOLOID, -1,
*isNull); *isNull);
return DatumGetBool(exprdatum); return DatumGetBool(exprdatum);
@ -4730,7 +4729,10 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
* through with found = false. * through with found = false.
*/ */
if (n <= 0) if (n <= 0)
{
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc); exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
exec_eval_cleanup(estate);
}
else else
found = true; /* processed at least one tuple */ found = true; /* processed at least one tuple */
@ -4747,6 +4749,7 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
* Assign the tuple to the target * Assign the tuple to the target
*/ */
exec_move_row(estate, rec, row, tuptab->vals[i], tuptab->tupdesc); exec_move_row(estate, rec, row, tuptab->vals[i], tuptab->tupdesc);
exec_eval_cleanup(estate);
/* /*
* Execute the statements * Execute the statements
@ -5138,6 +5141,9 @@ plpgsql_param_fetch(ParamListInfo params, int paramid)
/* ---------- /* ----------
* exec_move_row Move one tuple's values into a record or row * exec_move_row Move one tuple's values into a record or row
*
* Since this uses exec_assign_value, caller should eventually call
* exec_eval_cleanup to prevent long-term memory leaks.
* ---------- * ----------
*/ */
static void static void
@ -5338,28 +5344,44 @@ make_tuple_from_row(PLpgSQL_execstate *estate,
/* ---------- /* ----------
* convert_value_to_string Convert a non-null Datum to C string * convert_value_to_string Convert a non-null Datum to C string
* *
* Note: callers generally assume that the result is a palloc'd string and * Note: the result is in the estate's eval_econtext, and will be cleared
* should be pfree'd. This is not all that safe an assumption ... * by the next exec_eval_cleanup() call. The invoked output function might
* leave additional cruft there as well, so just pfree'ing the result string
* would not be enough to avoid memory leaks if we did not do it like this.
* In most usages the Datum being passed in is also in that context (if
* pass-by-reference) and so an exec_eval_cleanup() call is needed anyway.
* *
* Note: not caching the conversion function lookup is bad for performance. * Note: not caching the conversion function lookup is bad for performance.
* ---------- * ----------
*/ */
static char * static char *
convert_value_to_string(Datum value, Oid valtype) convert_value_to_string(PLpgSQL_execstate *estate, Datum value, Oid valtype)
{ {
char *result;
MemoryContext oldcontext;
Oid typoutput; Oid typoutput;
bool typIsVarlena; bool typIsVarlena;
oldcontext = MemoryContextSwitchTo(estate->eval_econtext->ecxt_per_tuple_memory);
getTypeOutputInfo(valtype, &typoutput, &typIsVarlena); getTypeOutputInfo(valtype, &typoutput, &typIsVarlena);
return OidOutputFunctionCall(typoutput, value); result = OidOutputFunctionCall(typoutput, value);
MemoryContextSwitchTo(oldcontext);
return result;
} }
/* ---------- /* ----------
* exec_cast_value Cast a value if required * exec_cast_value Cast a value if required
*
* Note: the estate's eval_econtext is used for temporary storage, and may
* also contain the result Datum if we have to do a conversion to a pass-
* by-reference data type. Be sure to do an exec_eval_cleanup() call when
* done with the result.
* ---------- * ----------
*/ */
static Datum static Datum
exec_cast_value(Datum value, Oid valtype, exec_cast_value(PLpgSQL_execstate *estate,
Datum value, Oid valtype,
Oid reqtype, Oid reqtype,
FmgrInfo *reqinput, FmgrInfo *reqinput,
Oid reqtypioparam, Oid reqtypioparam,
@ -5367,25 +5389,27 @@ exec_cast_value(Datum value, Oid valtype,
bool isnull) bool isnull)
{ {
/* /*
* If the type of the queries return value isn't that of the variable, * If the type of the given value isn't what's requested, convert it.
* convert it.
*/ */
if (valtype != reqtype || reqtypmod != -1) if (valtype != reqtype || reqtypmod != -1)
{ {
MemoryContext oldcontext;
oldcontext = MemoryContextSwitchTo(estate->eval_econtext->ecxt_per_tuple_memory);
if (!isnull) if (!isnull)
{ {
char *extval; char *extval;
extval = convert_value_to_string(value, valtype); extval = convert_value_to_string(estate, value, valtype);
value = InputFunctionCall(reqinput, extval, value = InputFunctionCall(reqinput, extval,
reqtypioparam, reqtypmod); reqtypioparam, reqtypmod);
pfree(extval);
} }
else else
{ {
value = InputFunctionCall(reqinput, NULL, value = InputFunctionCall(reqinput, NULL,
reqtypioparam, reqtypmod); reqtypioparam, reqtypmod);
} }
MemoryContextSwitchTo(oldcontext);
} }
return value; return value;
@ -5400,7 +5424,8 @@ exec_cast_value(Datum value, Oid valtype,
* ---------- * ----------
*/ */
static Datum static Datum
exec_simple_cast_value(Datum value, Oid valtype, exec_simple_cast_value(PLpgSQL_execstate *estate,
Datum value, Oid valtype,
Oid reqtype, int32 reqtypmod, Oid reqtype, int32 reqtypmod,
bool isnull) bool isnull)
{ {
@ -5414,7 +5439,8 @@ exec_simple_cast_value(Datum value, Oid valtype,
fmgr_info(typinput, &finfo_input); fmgr_info(typinput, &finfo_input);
value = exec_cast_value(value, value = exec_cast_value(estate,
value,
valtype, valtype,
reqtype, reqtype,
&finfo_input, &finfo_input,
@ -6137,7 +6163,10 @@ exec_dynquery_with_params(PLpgSQL_execstate *estate,
errmsg("query string argument of EXECUTE is null"))); errmsg("query string argument of EXECUTE is null")));
/* Get the C-String representation */ /* Get the C-String representation */
querystr = convert_value_to_string(query, restype); querystr = convert_value_to_string(estate, query, restype);
/* copy it out of the temporary context before we clean up */
querystr = pstrdup(querystr);
exec_eval_cleanup(estate); exec_eval_cleanup(estate);