From 931bbbd54de42861627cd1746718d239cd7b29f1 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 31 Aug 2016 16:44:23 +0900 Subject: [PATCH 1/2] PL/Tcl: Replace use of malloc by an internal memory context This simplifies the code to not rely anymore on malloc() calls and removes an old tweak to cache function information in the top memory context of a backend. --- src/pl/tcl/pltcl.c | 82 +++++++++++------------------------------------------- 1 file changed, 17 insertions(+), 65 deletions(-) diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c index 2a335aa..70e158b 100644 --- a/src/pl/tcl/pltcl.c +++ b/src/pl/tcl/pltcl.c @@ -313,23 +313,6 @@ pltcl_WaitForEvent(CONST86 Tcl_Time *timePtr) /* - * This routine is a crock, and so is everyplace that calls it. The problem - * is that the cached form of pltcl functions/queries is allocated permanently - * (mostly via malloc()) and never released until backend exit. Subsidiary - * data structures such as fmgr info records therefore must live forever - * as well. A better implementation would store all this stuff in a per- - * function memory context that could be reclaimed at need. In the meantime, - * fmgr_info_cxt must be called specifying TopMemoryContext so that whatever - * it might allocate, and whatever the eventual function might allocate using - * fn_mcxt, will live forever too. - */ -static void -perm_fmgr_info(Oid functionId, FmgrInfo *finfo) -{ - fmgr_info_cxt(functionId, finfo, TopMemoryContext); -} - -/* * _PG_init() - library load-time initialization * * DO NOT make this static nor change its name! @@ -1243,12 +1226,14 @@ static pltcl_proc_desc * compile_pltcl_function(Oid fn_oid, Oid tgreloid, bool is_event_trigger, bool pltrusted) { + volatile MemoryContext plan_cxt = NULL; HeapTuple procTup; Form_pg_proc procStruct; pltcl_proc_key proc_key; pltcl_proc_ptr *proc_ptr; bool found; pltcl_proc_desc *prodesc; + MemoryContext oldcontext = CurrentMemoryContext; /* We'll need the pg_proc tuple in any case... */ procTup = SearchSysCache1(PROCOID, ObjectIdGetDatum(fn_oid)); @@ -1330,21 +1315,21 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid, /************************************************************ * Allocate a new procedure description block + * + * struct prodesc and subsidiary data all live in plan_cxt. ************************************************************/ - prodesc = (pltcl_proc_desc *) malloc(sizeof(pltcl_proc_desc)); - if (prodesc == NULL) - ereport(ERROR, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); - MemSet(prodesc, 0, sizeof(pltcl_proc_desc)); - prodesc->user_proname = strdup(NameStr(procStruct->proname)); - prodesc->internal_proname = strdup(internal_proname); - if (prodesc->user_proname == NULL || prodesc->internal_proname == NULL) - ereport(ERROR, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + plan_cxt = AllocSetContextCreate(TopMemoryContext, + "PL/TCL compile", + ALLOCSET_SMALL_MINSIZE, + ALLOCSET_SMALL_INITSIZE, + ALLOCSET_SMALL_MAXSIZE); + MemoryContextSwitchTo(plan_cxt); + prodesc = (pltcl_proc_desc *) palloc0(sizeof(pltcl_proc_desc)); + prodesc->user_proname = pstrdup(NameStr(procStruct->proname)); + prodesc->internal_proname = pstrdup(internal_proname); prodesc->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data); prodesc->fn_tid = procTup->t_self; + MemoryContextSwitchTo(oldcontext); /* Remember if function is STABLE/IMMUTABLE */ prodesc->fn_readonly = @@ -1368,13 +1353,8 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid, SearchSysCache1(TYPEOID, ObjectIdGetDatum(procStruct->prorettype)); if (!HeapTupleIsValid(typeTup)) - { - free(prodesc->user_proname); - free(prodesc->internal_proname); - free(prodesc); elog(ERROR, "cache lookup failed for type %u", procStruct->prorettype); - } typeStruct = (Form_pg_type) GETSTRUCT(typeTup); /* Disallow pseudotype result, except VOID */ @@ -1384,37 +1364,22 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid, /* okay */ ; else if (procStruct->prorettype == TRIGGEROID || procStruct->prorettype == EVTTRIGGEROID) - { - free(prodesc->user_proname); - free(prodesc->internal_proname); - free(prodesc); ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("trigger functions can only be called as triggers"))); - } else - { - free(prodesc->user_proname); - free(prodesc->internal_proname); - free(prodesc); ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("PL/Tcl functions cannot return type %s", format_type_be(procStruct->prorettype)))); - } } if (typeStruct->typtype == TYPTYPE_COMPOSITE) - { - free(prodesc->user_proname); - free(prodesc->internal_proname); - free(prodesc); ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("PL/Tcl functions cannot return composite types"))); - } - perm_fmgr_info(typeStruct->typinput, &(prodesc->result_in_func)); + fmgr_info_cxt(typeStruct->typinput, &(prodesc->result_in_func), plan_cxt); prodesc->result_typioparam = getTypeIOParam(typeTup); ReleaseSysCache(typeTup); @@ -1433,26 +1398,16 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid, typeTup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(procStruct->proargtypes.values[i])); if (!HeapTupleIsValid(typeTup)) - { - free(prodesc->user_proname); - free(prodesc->internal_proname); - free(prodesc); elog(ERROR, "cache lookup failed for type %u", procStruct->proargtypes.values[i]); - } typeStruct = (Form_pg_type) GETSTRUCT(typeTup); /* Disallow pseudotype argument */ if (typeStruct->typtype == TYPTYPE_PSEUDO) - { - free(prodesc->user_proname); - free(prodesc->internal_proname); - free(prodesc); ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("PL/Tcl functions cannot accept type %s", format_type_be(procStruct->proargtypes.values[i])))); - } if (typeStruct->typtype == TYPTYPE_COMPOSITE) { @@ -1462,8 +1417,8 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid, else { prodesc->arg_is_rowtype[i] = false; - perm_fmgr_info(typeStruct->typoutput, - &(prodesc->arg_out_func[i])); + fmgr_info_cxt(typeStruct->typoutput, + &(prodesc->arg_out_func[i]), plan_cxt); snprintf(buf, sizeof(buf), "%d", i + 1); } @@ -1568,9 +1523,6 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid, Tcl_DStringFree(&proc_internal_def); if (tcl_rc != TCL_OK) { - free(prodesc->user_proname); - free(prodesc->internal_proname); - free(prodesc); ereport(ERROR, (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION), errmsg("could not create internal procedure \"%s\": %s", -- 2.9.3