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

Commit 0fe397f

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 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
1 parent 3ae9d4d commit 0fe397f

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
@@ -183,6 +183,7 @@ typedef struct plperl_call_data
183183
typedef struct plperl_query_desc
184184
{
185185
char qname[24];
186+
MemoryContext plan_cxt; /* context holding this struct */
186187
SPIPlanPtr plan;
187188
int nargs;
188189
Oid *argtypes;
@@ -3200,33 +3201,57 @@ plperl_spi_cursor_close(char *cursor)
32003201
SV *
32013202
plperl_spi_prepare(char *query, int argc, SV **argv)
32023203
{
3203-
plperl_query_desc *qdesc;
3204-
plperl_query_entry *hash_entry;
3205-
bool found;
3206-
SPIPlanPtr plan;
3207-
int i;
3208-
3204+
volatile SPIPlanPtr plan = NULL;
3205+
volatile MemoryContext plan_cxt = NULL;
3206+
plperl_query_desc *volatile qdesc = NULL;
3207+
plperl_query_entry *volatile hash_entry = NULL;
32093208
MemoryContext oldcontext = CurrentMemoryContext;
32103209
ResourceOwner oldowner = CurrentResourceOwner;
3210+< 10000 div class="diff-text-inner"> MemoryContext work_cxt;
3211+
bool found;
3212+
int i;
32113213

32123214
check_spi_usage_allowed();
32133215

32143216
BeginInternalSubTransaction(NULL);
32153217
MemoryContextSwitchTo(oldcontext);
32163218

3217-
/************************************************************
3218-
* Allocate the new querydesc structure
3219-
************************************************************/
3220-
qdesc = (plperl_query_desc *) malloc(sizeof(plperl_query_desc));
3221-
MemSet(qdesc, 0, sizeof(plperl_query_desc));
3222-
snprintf(qdesc->qname, sizeof(qdesc->qname), "%p", qdesc);
3223-
qdesc->nargs = argc;
3224-
qdesc->argtypes = (Oid *) malloc(argc * sizeof(Oid));
3225-
qdesc->arginfuncs = (FmgrInfo *) malloc(argc * sizeof(FmgrInfo));
3226-
qdesc->argtypioparams = (Oid *) malloc(argc * sizeof(Oid));
3227-
32283219
PG_TRY();
32293220
{
3221+
CHECK_FOR_INTERRUPTS();
3222+
3223+
/************************************************************
3224+
* Allocate the new querydesc structure
3225+
*
3226+
* The qdesc struct, as well as all its subsidiary data, lives in its
3227+
* plan_cxt. But note that the SPIPlan does not.
3228+
************************************************************/
3229+
plan_cxt = AllocSetContextCreate(TopMemoryContext,
3230+
"PL/Perl spi_prepare query",
3231+
ALLOCSET_SMALL_MINSIZE,
3232+
ALLOCSET_SMALL_INITSIZE,
3233+
ALLOCSET_SMALL_MAXSIZE);
3234+
MemoryContextSwitchTo(plan_cxt);
3235+
qdesc = (plperl_query_desc *) palloc0(sizeof(plperl_query_desc));
3236+
snprintf(qdesc->qname, sizeof(qdesc->qname), "%p", qdesc);
3237+
qdesc->plan_cxt = plan_cxt;
3238+
qdesc->nargs = argc;
3239+
qdesc->argtypes = (Oid *) palloc(argc * sizeof(Oid));
3240+
qdesc->arginfuncs = (FmgrInfo *) palloc(argc * sizeof(FmgrInfo));
3241+
qdesc->argtypioparams = (Oid *) palloc(argc * sizeof(Oid));
3242+
MemoryContextSwitchTo(oldcontext);
3243+
3244+
/************************************************************
3245+
* Do the following work in a short-lived context so that we don't
3246+
* leak a lot of memory in the PL/Perl function's SPI Proc context.
3247+
************************************************************/
3248+
work_cxt = AllocSetContextCreate(CurrentMemoryContext,
3249+
"PL/Perl spi_prepare workspace",
3250+
ALLOCSET_DEFAULT_MINSIZE,
3251+
ALLOCSET_DEFAULT_INITSIZE,
3252+
ALLOCSET_DEFAULT_MAXSIZE);
3253+
MemoryContextSwitchTo(work_cxt);
3254+
32303255
/************************************************************
32313256
* Resolve argument type names and then look them up by oid
32323257
* in the system cache, and remember the required information
@@ -3247,7 +3272,7 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
32473272
getTypeInputInfo(typId, &typInput, &typIOParam);
32483273

32493274
qdesc->argtypes[i] = typId;
3250-
perm_fmgr_info(typInput, &(qdesc->arginfuncs[i]));
3275+
fmgr_info_cxt(typInput, &(qdesc->arginfuncs[i]), plan_cxt);
32513276
qdesc->argtypioparams[i] = typIOParam;
32523277
}
32533278

@@ -3271,6 +3296,17 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
32713296
elog(ERROR, "SPI_keepplan() failed");
32723297
qdesc->plan = plan;
32733298

3299+
/************************************************************
3300+
* Insert a hashtable entry for the plan.
3301+
************************************************************/
3302+
hash_entry = hash_search(plperl_active_interp->query_hash,
3303+
qdesc->qname,
3304+
HASH_ENTER, &found);
3305+
hash_entry->query_data = qdesc;
3306+
3307+
/* Get rid of workspace */
3308+
MemoryContextDelete(work_cxt);
3309+
32743310
/* Commit the inner transaction, return to outer xact context */
32753311
ReleaseCurrentSubTransaction();
32763312
MemoryContextSwitchTo(oldcontext);
@@ -3286,16 +3322,21 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
32863322
{
32873323
ErrorData *edata;
32883324

3289-
free(qdesc->argtypes);
3290-
free(qdesc->arginfuncs);
3291-
free(qdesc->argtypioparams);
3292-
free(qdesc);
3293-
32943325
/* Save error info */
32953326
MemoryContextSwitchTo(oldcontext);
32963327
edata = CopyErrorData();
32973328
FlushErrorState();
32983329

3330+
/* Drop anything we managed to allocate */
3331+
if (hash_entry)
3332+
hash_search(plperl_active_interp->query_hash,
3333+
qdesc->qname,
3334+
HASH_REMOVE, NULL);
3335+
if (plan_cxt)
3336+
MemoryContextDelete(plan_cxt);
3337+
if (plan)
3338+
SPI_freeplan(plan);
3339+
32993340
/* Abort the inner transaction */
33003341
RollbackAndReleaseCurrentSubTransaction();
33013342
MemoryContextSwitchTo(oldcontext);
@@ -3317,14 +3358,8 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
33173358
PG_END_TRY();
33183359

33193360
/************************************************************
3320-
* Insert a hashtable entry for the plan and return
3321-
* the key to the caller.
3361+
* Return the query's hash key to the caller.
33223362
************************************************************/
3323-
3324-
hash_entry = hash_search(plperl_active_interp->query_hash, qdesc->qname,
3325-
HASH_ENTER, &found);
3326-
hash_entry->query_data = qdesc;
3327-
33283363
return cstr2sv(qdesc->qname);
33293364
}
33303365

@@ -3359,16 +3394,14 @@ plperl_spi_exec_prepared(char *query, HV *attr, int argc, SV **argv)
33593394
/************************************************************
33603395
* Fetch the saved plan descriptor, see if it's o.k.
33613396
************************************************************/
3362-
33633397
hash_entry = hash_search(plperl_active_interp->query_hash, query,
33643398
HASH_FIND, NULL);
33653399
if (hash_entry == NULL)
33663400
elog(ERROR, "spi_exec_prepared: Invalid prepared query passed");
33673401

33683402
qdesc = hash_entry->query_data;
3369-
33703403
if (qdesc == NULL)
3371-
elog(ERROR, "spi_exec_prepared: panic - plperl query_hash value vanished");
3404+
elog(ERROR, "spi_exec_prepared: plperl query_hash value vanished");
33723405

33733406
if (qdesc->nargs != argc)
33743407
elog(ERROR, "spi_exec_prepared: expected %d argument(s), %d passed",
@@ -3500,12 +3533,11 @@ plperl_spi_query_prepared(char *query, int argc, SV **argv)
35003533
hash_entry = hash_search(plperl_active_interp->query_hash, query,
35013534
HASH_FIND, NULL);
35023535
if (hash_entry == NULL)
3503-
elog(ERROR, "spi_exec_prepared: Invalid prepared query passed");
3536+
elog(ERROR, "spi_query_prepared: Invalid prepared query passed");
35043537

35053538
qdesc = hash_entry->query_data;
3506-
35073539
if (qdesc == NULL)
3508-
elog(ERROR, "spi_query_prepared: panic - plperl query_hash value vanished");
3540+
elog(ERROR, "spi_query_prepared: plperl query_hash value vanished");
35093541

35103542
if (qdesc->nargs != argc)
35113543
elog(ERROR, "spi_query_prepared: expected %d argument(s), %d passed",
@@ -3610,12 +3642,12 @@ plperl_spi_freeplan(char *query)
36103642
hash_entry = hash_search(plperl_active_interp->query_hash, query,
36113643
HASH_FIND, NULL);
36123644
if (hash_entry == NULL)
3613-
elog(ERROR, "spi_exec_prepared: Invalid prepared query passed");
3645+
elog(ERROR, "spi_freeplan: Invalid prepared query passed");
36143646

36153647
qdesc = hash_entry->query_data;
3616-
36173648
if (qdesc == NULL)
3618-
elog(ERROR, "spi_exec_freeplan: panic - plperl query_hash value vanished");
3649+
elog(ERROR, "spi_freeplan: plperl query_hash value vanished");
3650+
plan = qdesc->plan;
36193651

36203652
/*
36213653
* free all memory before SPI_freeplan, so if it dies, nothing will be
@@ -3624,11 +3656,7 @@ plperl_spi_freeplan(char *query)
36243656
hash_search(plperl_active_interp->query_hash, query,
36253657
HASH_REMOVE, NULL);
36263658

3627-
plan = qdesc->plan;
3628-
free(qdesc->argtypes);
3629-
free(qdesc->arginfuncs);
3630-
free(qdesc->argtypioparams);
3631-
free(qdesc);
3659+
MemoryContextDelete(qdesc->plan_cxt);
36323660

36333661
SPI_freeplan(plan);
36343662
}

0 commit comments

Comments
 (0)
0