database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-02 09:02:37 +03:00
Code Activity

Eliminate memory leaks in plperl's spi_prepare() function.

Browse Source 
Careless use of TopMemoryContext for I/O function data meant that repeated
use of spi_prepare and spi_freeplan would leak memory at the session level,
as per report from Christian Schröder.  In addition, spi_prepare
leaked a lot of transient data within the current plperl function's SPI
Proc context, which would be a problem for repeated use of spi_prepare
within a single plperl function call; and it wasn't terribly careful
about releasing permanent allocations in event of an error, either.

In passing, clean up some copy-and-pasteos in query-lookup error messages.

Alex Hunsaker and Tom Lane
This commit is contained in:
Tom Lane
2013-03-01 21:33:48 -05:00
parent 4aa1546726
commit 698d99221d
1 changed files with 72 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

116
src/pl/plperl/plperl.c
View File

@ -175,6 +175,7 @@ typedef struct plperl_call_data
typedef struct plperl_query_desc
{
char qname[24];
MemoryContext plan_cxt; /* context holding this struct */
void *plan;
int nargs;
Oid *argtypes;
@ -2387,31 +2388,55 @@ plperl_spi_cursor_close(char *cursor)
SV *
plperl_spi_prepare(char *query, int argc, SV **argv)
{
plperl_query_desc *qdesc;
plperl_query_entry *hash_entry;
bool found;
void *plan;
int i;
void *volatile plan = NULL;
volatile MemoryContext plan_cxt = NULL;
plperl_query_desc *volatile qdesc = NULL;
plperl_query_entry *volatile hash_entry = NULL;
MemoryContext oldcontext = CurrentMemoryContext;
ResourceOwner oldowner = CurrentResourceOwner;
MemoryContext work_cxt;
bool found;
int i;
BeginInternalSubTransaction(NULL);
MemoryContextSwitchTo(oldcontext);
/************************************************************
* Allocate the new querydesc structure
************************************************************/
qdesc = (plperl_query_desc *) malloc(sizeof(plperl_query_desc));
MemSet(qdesc, 0, sizeof(plperl_query_desc));
snprintf(qdesc->qname, sizeof(qdesc->qname), "%lx", (long) qdesc);
qdesc->nargs = argc;
qdesc->argtypes = (Oid *) malloc(argc * sizeof(Oid));
qdesc->arginfuncs = (FmgrInfo *) malloc(argc * sizeof(FmgrInfo));
qdesc->argtypioparams = (Oid *) malloc(argc * sizeof(Oid));
PG_TRY();
{
CHECK_FOR_INTERRUPTS();
/************************************************************
* Allocate the new querydesc structure
*
* The qdesc struct, as well as all its subsidiary data, lives in its
* plan_cxt. But note that the SPIPlan does not.
************************************************************/
plan_cxt = AllocSetContextCreate(TopMemoryContext,
"PL/Perl spi_prepare query",
ALLOCSET_SMALL_MINSIZE,
ALLOCSET_SMALL_INITSIZE,
ALLOCSET_SMALL_MAXSIZE);
MemoryContextSwitchTo(plan_cxt);
qdesc = (plperl_query_desc *) palloc0(sizeof(plperl_query_desc));
snprintf(qdesc->qname, sizeof(qdesc->qname), "%lx", (long) qdesc);
qdesc->plan_cxt = plan_cxt;
qdesc->nargs = argc;
qdesc->argtypes = (Oid *) palloc(argc * sizeof(Oid));
qdesc->arginfuncs = (FmgrInfo *) palloc(argc * sizeof(FmgrInfo));
qdesc->argtypioparams = (Oid *) palloc(argc * sizeof(Oid));
MemoryContextSwitchTo(oldcontext);
/************************************************************
* Do the following work in a short-lived context so that we don't
* leak a lot of memory in the PL/Perl function's SPI Proc context.
************************************************************/
work_cxt = AllocSetContextCreate(CurrentMemoryContext,
"PL/Perl spi_prepare workspace",
ALLOCSET_DEFAULT_MINSIZE,
ALLOCSET_DEFAULT_INITSIZE,
ALLOCSET_DEFAULT_MAXSIZE);
MemoryContextSwitchTo(work_cxt);
/************************************************************
* Resolve argument type names and then look them up by oid
* in the system cache, and remember the required information
@ -2429,7 +2454,7 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
getTypeInputInfo(typId, &typInput, &typIOParam);
qdesc->argtypes[i] = typId;
perm_fmgr_info(typInput, &(qdesc->arginfuncs[i]));
fmgr_info_cxt(typInput, &(qdesc->arginfuncs[i]), plan_cxt);
qdesc->argtypioparams[i] = typIOParam;
}
@ -2454,6 +2479,17 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
/* Release the procCxt copy to avoid within-function memory leak */
SPI_freeplan(plan);
/************************************************************
* Insert a hashtable entry for the plan.
************************************************************/
hash_entry = hash_search(plperl_active_interp->query_hash,
qdesc->qname,
HASH_ENTER, &found);
hash_entry->query_data = qdesc;
/* Get rid of workspace */
MemoryContextDelete(work_cxt);
/* Commit the inner transaction, return to outer xact context */
ReleaseCurrentSubTransaction();
MemoryContextSwitchTo(oldcontext);
@ -2469,16 +2505,21 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
{
ErrorData *edata;
free(qdesc->argtypes);
free(qdesc->arginfuncs);
free(qdesc->argtypioparams);
free(qdesc);
/* Save error info */
MemoryContextSwitchTo(oldcontext);
edata = CopyErrorData();
FlushErrorState();
/* Drop anything we managed to allocate */
if (hash_entry)
hash_search(plperl_active_interp->query_hash,
qdesc->qname,
HASH_REMOVE, NULL);
if (plan_cxt)
MemoryContextDelete(plan_cxt);
if (plan)
SPI_freeplan(plan);
/* Abort the inner transaction */
RollbackAndReleaseCurrentSubTransaction();
MemoryContextSwitchTo(oldcontext);
@ -2500,14 +2541,8 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
PG_END_TRY();
/************************************************************
* Insert a hashtable entry for the plan and return
* the key to the caller.
* Return the query's hash key to the caller.
************************************************************/
hash_entry = hash_search(plperl_active_interp->query_hash, qdesc->qname,
HASH_ENTER, &found);
hash_entry->query_data = qdesc;
return newSVstring(qdesc->qname);
}
@ -2540,16 +2575,14 @@ plperl_spi_exec_prepared(char *query, HV *attr, int argc, SV **argv)
/************************************************************
* Fetch the saved plan descriptor, see if it's o.k.
************************************************************/
hash_entry = hash_search(plperl_active_interp->query_hash, query,
HASH_FIND, NULL);
if (hash_entry == NULL)
elog(ERROR, "spi_exec_prepared: Invalid prepared query passed");
qdesc = hash_entry->query_data;
if (qdesc == NULL)
elog(ERROR, "spi_exec_prepared: panic - plperl query_hash value vanished");
elog(ERROR, "spi_exec_prepared: plperl query_hash value vanished");
if (qdesc->nargs != argc)
elog(ERROR, "spi_exec_prepared: expected %d argument(s), %d passed",
@ -2685,12 +2718,11 @@ plperl_spi_query_prepared(char *query, int argc, SV **argv)
hash_entry = hash_search(plperl_active_interp->query_hash, query,
HASH_FIND, NULL);
if (hash_entry == NULL)
elog(ERROR, "spi_exec_prepared: Invalid prepared query passed");
elog(ERROR, "spi_query_prepared: Invalid prepared query passed");
qdesc = hash_entry->query_data;
if (qdesc == NULL)
elog(ERROR, "spi_query_prepared: panic - plperl query_hash value vanished");
elog(ERROR, "spi_query_prepared: plperl query_hash value vanished");
if (qdesc->nargs != argc)
elog(ERROR, "spi_query_prepared: expected %d argument(s), %d passed",
@ -2799,12 +2831,12 @@ plperl_spi_freeplan(char *query)
hash_entry = hash_search(plperl_active_interp->query_hash, query,
HASH_FIND, NULL);
if (hash_entry == NULL)
elog(ERROR, "spi_exec_prepared: Invalid prepared query passed");
elog(ERROR, "spi_freeplan: Invalid prepared query passed");
qdesc = hash_entry->query_data;
if (qdesc == NULL)
elog(ERROR, "spi_exec_freeplan: panic - plperl query_hash value vanished");
elog(ERROR, "spi_freeplan: plperl query_hash value vanished");
plan = qdesc->plan;
/*
* free all memory before SPI_freeplan, so if it dies, nothing will be
@ -2813,11 +2845,7 @@ plperl_spi_freeplan(char *query)
hash_search(plperl_active_interp->query_hash, query,
HASH_REMOVE, NULL);
plan = qdesc->plan;
free(qdesc->argtypes);
free(qdesc->arginfuncs);
free(qdesc->argtypioparams);
free(qdesc);
MemoryContextDelete(qdesc->plan_cxt);
SPI_freeplan(plan);
}