8000 Eliminate memory leaks in plperl's spi_prepare() function. · commandprompt/postgres@18c6dd8 · GitHub
[go: up one dir, main page]
Skip to content

Commit 18c6dd8

Browse files
tglsfdctglsfdc
committed
Eliminate memory leaks in plperl's spi_prepare() function.
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 tra 8000 nsient 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
1 parent 0c54796 commit 18c6dd8

File tree

1 file changed

+72
-44
lines changed

1 file changed

+72
-44
lines changed

‎src/pl/plperl/plperl.c

Lines changed: 72 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ typedef struct plperl_call_data
180180
typedef struct plperl_query_desc
181181
{
182182
char qname[24];
183+
MemoryContext plan_cxt; /* context holding this struct */
183184
void *plan;
184185
int nargs;
185186
Oid *argtypes;
@@ -2716,33 +2717,57 @@ plperl_spi_cursor_close(char *cursor)
27162717
SV *
27172718
plperl_spi_prepare(char *query, int argc, SV **argv)
27182719
{
2719-
plperl_query_desc *qdesc;
2720-
plperl_query_entry *hash_entry;
2721-
bool found;
2722-
void *plan;
2723-
int i;
2724-
2720+
void *volatile plan = NULL;
2721+
volatile MemoryContext plan_cxt = NULL;
2722+
plperl_query_desc *volatile qdesc = NULL;
2723+
plperl_query_entry *volatile hash_entry = NULL;
27252724
MemoryContext oldcontext = CurrentMemoryContext;
27262725
ResourceOwner oldowner = CurrentResourceOwner;
2726+
MemoryContext work_cxt;
2727+
bool found;
2728+
int i;
27272729

27282730
check_spi_usage_allowed();
27292731

27302732
BeginInternalSubTransaction(NULL);
27312733
MemoryContextSwitchTo(oldcontext);
27322734

2733-
/************************************************************
2734-
* Allocate the new querydesc structure
2735-
************************************************************/
2736-
qdesc = (plperl_query_desc *) malloc(sizeof(plperl_query_desc));
2737-
MemSet(qdesc, 0, sizeof(plperl_query_desc));
2738-
snprintf(qdesc->qname, sizeof(qdesc->qname), "%p", qdesc);
2739-
qdesc->nargs = argc;
2740-
qdesc->argtypes = (Oid *) malloc(argc * sizeof(Oid));
2741-
qdesc->arginfuncs = (FmgrInfo *) malloc(argc * sizeof(FmgrInfo));
2742-
qdesc->argtypioparams = (Oid *) malloc(argc * sizeof(Oid));
2743-
27442735
PG_TRY();
27452736
{
2737+
CHECK_FOR_INTERRUPTS();
2738+
2739+
/************************************************************
2740+
* Allocate the new querydesc structure
2741+
*
2742+
* The qdesc struct, as well as all its subsidiary data, lives in its
2743+
* plan_cxt. But note that the SPIPlan does not.
2744+
************************************************************/
2745+
plan_cxt = AllocSetContextCreate(TopMemoryContext,
2746+
"PL/Perl spi_prepare query",
2747+
ALLOCSET_SMALL_MINSIZE,
2748+
ALLOCSET_SMALL_INITSIZE,
2749+
ALLOCSET_SMALL_MAXSIZE);
2750+
MemoryContextSwitchTo(plan_cxt);
2751+
qdesc = (plperl_query_desc *) palloc0(sizeof(plperl_query_desc));
2752+
snprintf(qdesc->qname, sizeof(qdesc->qname), "%p", qdesc);
2753+
qdesc->plan_cxt = plan_cxt;
2754+
qdesc->nargs = argc;
2755+
qdesc->argtypes = (Oid *) palloc(argc * sizeof(Oid));
2756+
qdesc->arginfuncs = (FmgrInfo *) palloc(argc * sizeof(FmgrInfo));
2757+
qdesc->argtypioparams = (Oid *) palloc(argc * sizeof(Oid));
2758+
MemoryContextSwitchTo(oldcontext);
2759+
2760+
/************************************************************
2761+
* Do the following work in a short-lived context so that we don't
2762+
* leak a lot of memory in the PL/Perl function's SPI Proc context.
2763+
************************************************************/
2764+
work_cxt = AllocSetContextCreate(CurrentMemoryContext,
2765+
"PL/Perl spi_prepare workspace",
2766+
ALLOCSET_DEFAULT_MINSIZE,
2767+
ALLOCSET_DEFAULT_INITSIZE,
2768+
ALLOCSET_DEFAULT_MAXSIZE);
2769+
MemoryContextSwitchTo(work_cxt);
2770+
27462771
/************************************************************
27472772
* Resolve argument type names and then look them up by oid
27482773
* in the system cache, and remember the required information
@@ -2760,7 +2785,7 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
27602785
getTypeInputInfo(typId, &typInput, &typIOParam);
27612786

27622787
qdesc->argtypes[i] = typId;
2763-
perm_fmgr_info(typInput, &(qdesc->arginfuncs[i]));
2788+
fmgr_info_cxt(typInput, &(qdesc->arginfuncs[i]), plan_cxt);
27642789
qdesc->argtypioparams[i] = typIOParam;
27652790
}
27662791

@@ -2788,6 +2813,17 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
27882813
/* Release the procCxt copy to avoid within-function memory leak */
27892814
SPI_freeplan(plan);
27902815

2816+
/************************************************************
2817+
* Insert a hashtable entry for the plan.
2818+
************************************************************/
2819+
hash_entry = hash_search(plperl_active_interp->query_hash,
2820+
qdesc->qname,
2821+
HASH_ENTER, &found);
2822+
hash_entry->query_data = qdesc;
2823+
2824+
/* Get rid of workspace */
2825+
MemoryContextDelete(work_cxt);
2826+
27912827
/* Commit the inner transaction, return to outer xact context */
27922828
ReleaseCurrentSubTransaction();
27932829
MemoryContextSwitchTo(oldcontext);
@@ -2803,16 +2839,21 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
28032839
{
28042840
ErrorData *edata;
28052841

2806-
free(qdesc->argtypes);
2807-
free(qdesc->arginfuncs);
2808-
free(qdesc->argtypioparams);
2809-
free(qdesc);
2810-
28112842
/* Save error info */
28122843
MemoryContextSwitchTo(oldcontext);
28132844
edata = CopyErrorData();
28142845
FlushErrorState();
28152846

2847+
/* Drop anything we managed to allocate */
2848+
if (hash_entry)
2849+
hash_search(plperl_active_interp->query_hash,
2850+
qdesc->qname,
2851+
HASH_REMOVE, NULL);
2852+
if (plan_cxt)
2853+
MemoryContextDelete(plan_cxt);
2854+
if (plan)
2855+
SPI_freeplan(plan);
2856+
28162857
/* Abort the inner transaction */
28172858
RollbackAndReleaseCurrentSubTransaction();
28182859
MemoryContextSwitchTo(oldcontext);
@@ -2834,14 +2875,8 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
28342875
PG_END_TRY();
28352876

28362877
/************************************************************
2837-
* Insert a hashtable entry for the plan and return
2838-
* the key to the caller.
2878+
* Return the query's hash key to the caller.
28392879
************************************************************/
2840-
2841-
hash_entry = hash_search(plperl_active_interp->query_hash, qdesc->qname,
2842-
HASH_ENTER, &found);
2843-
hash_entry->query_data = qdesc;
2844-
28452880
return newSVstring(qdesc->qname);
28462881
}
28472882

@@ -2876,16 +2911,14 @@ plperl_spi_exec_prepared(char *query, HV *attr, int argc, SV **argv)
28762911
/************************************************************
28772912
* Fetch the saved plan descriptor, see if it's o.k.
28782913
************************************************************/
2879-
28802914
hash_entry = hash_search(plperl_active_interp->query_hash, query,
28812915
HASH_FIND, NULL);
28822916
if (hash_entry == NULL)
28832917
elog(ERROR, "spi_exec_prepared: Invalid prepared query passed");
28842918

28852919
qdesc = hash_entry->query_data;
2886-
28872920
if (qdesc == NULL)
2888-
elog(ERROR, "spi_exec_prepared: panic - plperl query_hash value vanished");
2921+
elog(ERROR, "spi_exec_prepared: plperl query_hash value vanished");
28892922

28902923
if (qdesc->nargs != argc)
28912924
elog(ERROR, "spi_exec_prepared: expected %d argument(s), %d passed",
@@ -3023,12 +3056,11 @@ plperl_spi_query_prepared(char *query, int argc, SV **argv)
30233056
hash_entry = hash_search(plperl_active_interp->query_hash, query,
30243057
HASH_FIND, NULL);
30253058
if (hash_entry == NULL)
3026-
elog(ERROR, "spi_exec_prepared: Invalid prepared query passed");
3059+
elog(ERROR, "spi_query_prepared: Invalid prepared query passed");
30273060

30283061
qdesc = hash_entry->query_data;
3029-
30303062
if (qdesc == NULL)
3031-
elog(ERROR, "spi_query_prepared: panic - plperl query_hash value vanished");
3063+
elog(ERROR, "spi_query_prepared: plperl query_hash value vanished");
30323064

30333065
if (qdesc->nargs != argc)
30343066
elog(ERROR, "spi_query_prepared: expected %d argument(s), %d passed",
@@ -3139,12 +3171,12 @@ plperl_spi_freeplan(char *query)
31393171
hash_entry = hash_search(plperl_active_interp->query_hash, query,
31403172
HASH_FIND, NULL);
31413173
if (hash_entry == NULL)
3142-
elog(ERROR, "spi_exec_prepared: Invalid prepared query passed");
3174+
elog(ERROR, "spi_freeplan: Invalid prepared query passed");
31433175

31443176
qdesc = hash_entry->query_data;
3145-
31463177
if (qdesc == NULL)
3147-
elog(ERROR, "spi_exec_freeplan: panic - plperl query_hash value vanished");
3178+
elog(ERROR, "spi_freeplan: plperl query_hash value vanished");
3179+
plan = qdesc->plan;
31483180

31493181
/*
31503182
* free all memory before SPI_freeplan, so if it dies, nothing will be
@@ -3153,11 +3185,7 @@ plperl_spi_freeplan(char *query)
31533185
hash_search(plperl_active_interp->query_hash, query,
31543186
HASH_REMOVE, NULL);
31553187

3156-
plan = qdesc->plan;
3157-
free(qdesc->argtypes);
3158-
free(qdesc->arginfuncs);
3159-
free(qdesc->argtypioparams);
3160-
free(qdesc);
3188+
MemoryContextDelete(qdesc->plan_cxt);
31613189

31623190
SPI_freeplan(plan);
31633191
}

0 commit comments

Comments
 (0)
0