Reduce leakage during PL/pgSQL function compilation.

format_procedure leaks memory, so run it in a short-lived context
not the session-lifespan cache context for the PL/pgSQL function.

parse_datatype called the core parser in the function's cache context,
thus leaking potentially a lot of storage into that context.  We were
also being a bit careless with the TypeName structures made in that
code path and others.  Most of the time we don't need to retain the
TypeName, so make sure it is made in the short-lived temp context,
and copy it only if we do need to retain it.

These are far from the only leaks in PL/pgSQL compilation, but
they're the biggest as far as I've seen, and further improvement
looks like it'd require delicate and bug-prone surgery.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index ee961425a5b7e9f59c68c3eb3362ffb4adafba87..f6976689a69279d19e1d9a9f713d4793f95ddadb 100644 (file)
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -177,6 +177,7 @@ plpgsql_compile_callback(FunctionCallInfo fcinfo,
        yyscan_t        scanner;
        Datum           prosrcdatum;
        char       *proc_source;
+       char       *proc_signature;
        HeapTuple       typeTup;
        Form_pg_type typeStruct;
        PLpgSQL_variable *var;
@@ -223,6 +224,9 @@ plpgsql_compile_callback(FunctionCallInfo fcinfo,
        plpgsql_check_syntax = forValidator;
        plpgsql_curr_compile = function;
 
+       /* format_procedure leaks memory, so run it in temp context */
+       proc_signature = format_procedure(fcinfo->flinfo->fn_oid);
+
        /*
         * All the permanent output of compilation (e.g. parse tree) is kept in a
         * per-function memory context, so it can be reclaimed easily.
@@ -237,7 +241,7 @@ plpgsql_compile_callback(FunctionCallInfo fcinfo,
                                                                         ALLOCSET_DEFAULT_SIZES);
        plpgsql_compile_tmp_cxt = MemoryContextSwitchTo(func_cxt);
 
-       function->fn_signature = format_procedure(fcinfo->flinfo->fn_oid);
+       function->fn_signature = pstrdup(proc_signature);
        MemoryContextSetIdentifier(func_cxt, function->fn_signature);
        function->fn_oid = fcinfo->flinfo->fn_oid;
        function->fn_input_collation = fcinfo->fncollation;
@@ -1673,6 +1677,11 @@ plpgsql_parse_wordrowtype(char *ident)
 {
        Oid                     classOid;
        Oid                     typOid;
+       TypeName   *typName;
+       MemoryContext oldCxt;
+
+       /* Avoid memory leaks in long-term function context */
+       oldCxt = MemoryContextSwitchTo(plpgsql_compile_tmp_cxt);
 
        /*
         * Look up the relation.  Note that because relation rowtypes have the
@@ -1695,9 +1704,12 @@ plpgsql_parse_wordrowtype(char *ident)
                                 errmsg("relation \"%s\" does not have a composite type",
                                                ident)));
 
+       typName = makeTypeName(ident);
+
+       MemoryContextSwitchTo(oldCxt);
+
        /* Build and return the row type struct */
-       return plpgsql_build_datatype(typOid, -1, InvalidOid,
-                                                                 makeTypeName(ident));
+       return plpgsql_build_datatype(typOid, -1, InvalidOid, typName);
 }
 
 /* ----------
@@ -1711,6 +1723,7 @@ plpgsql_parse_cwordrowtype(List *idents)
        Oid                     classOid;
        Oid                     typOid;
        RangeVar   *relvar;
+       TypeName   *typName;
        MemoryContext oldCxt;
 
        /*
@@ -1733,11 +1746,12 @@ plpgsql_parse_cwordrowtype(List *idents)
                                 errmsg("relation \"%s\" does not have a composite type",
                                                relvar->relname)));
 
+       typName = makeTypeNameFromNameList(idents);
+
        MemoryContextSwitchTo(oldCxt);
 
        /* Build and return the row type struct */
-       return plpgsql_build_datatype(typOid, -1, InvalidOid,
-                                                                 makeTypeNameFromNameList(idents));
+       return plpgsql_build_datatype(typOid, -1, InvalidOid, typName);
 }
 
 /*
@@ -1952,6 +1966,8 @@ plpgsql_build_recfield(PLpgSQL_rec *rec, const char *fldname)
  * origtypname is the parsed form of what the user wrote as the type name.
  * It can be NULL if the type could not be a composite type, or if it was
  * identified by OID to begin with (e.g., it's a function argument type).
+ * origtypname is in short-lived storage and must be copied if we choose
+ * to incorporate it into the function's parse tree.
  */
 PLpgSQL_type *
 plpgsql_build_datatype(Oid typeOid, int32 typmod,
@@ -2070,7 +2086,7 @@ build_datatype(HeapTuple typeTup, int32 typmod,
                                         errmsg("type %s is not composite",
                                                        format_type_be(typ->typoid))));
 
-               typ->origtypname = origtypname;
+               typ->origtypname = copyObject(origtypname);
                typ->tcache = typentry;
                typ->tupdesc_id = typentry->tupDesc_identifier;
        }

diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 7b672ea5179a61f6c439a505aebf056e01bc4889..17568d82554d21146573820ce0170b8201cb48a2 100644 (file)
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -3853,6 +3853,7 @@ parse_datatype(const char *string, int location, yyscan_t yyscanner)
        int32           typmod;
        sql_error_callback_arg cbarg;
        ErrorContextCallback syntax_errcontext;
+       MemoryContext oldCxt;
 
        cbarg.location = location;
        cbarg.yyscanner = yyscanner;
@@ -3862,9 +3863,14 @@ parse_datatype(const char *string, int location, yyscan_t yyscanner)
        syntax_errcontext.previous = error_context_stack;
        error_context_stack = &syntax_errcontext;
 
-       /* Let the main parser try to parse it under standard SQL rules */
+       /*
+        * Let the main parser try to parse it under standard SQL rules.  The
+        * parser leaks memory, so run it in temp context.
+        */
+       oldCxt = MemoryContextSwitchTo(plpgsql_compile_tmp_cxt);
        typeName = typeStringToTypeName(string, NULL);
        typenameTypeIdAndMod(NULL, typeName, &type_id, &typmod);
+       MemoryContextSwitchTo(oldCxt);
 
        /* Restore former ereport callback */
        error_context_stack = syntax_errcontext.previous;