Skip to content

Commit b2da7c8

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 f5185db commit b2da7c8

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
@@ -182,6 +182,7 @@ typedef struct plperl_call_data
182182
typedef struct plperl_query_desc
183183
{
184184
char qname[24];
185+
MemoryContext plan_cxt; /* context holding this struct */
185186
void *plan;
186187
int nargs;
187188
Oid *argtypes;
@@ -3199,33 +3200,57 @@ plperl_spi_cursor_close(char *cursor)
31993200
SV *
32003201
plperl_spi_prepare(char *query, int argc, SV **argv)
32013202
{
3202-
plperl_query_desc *qdesc;
3203-
plperl_query_entry *hash_entry;
3204-
bool found;
3205-
void *plan;
3206-
int i;
3207-
3203+
void *volatile plan = NULL;
3204+
volatile MemoryContext plan_cxt = NULL;
3205+
plperl_query_desc *volatile qdesc = NULL;
3206+
plperl_query_entry *volatile hash_entry = NULL;
32083207
MemoryContext oldcontext = CurrentMemoryContext;
32093208
ResourceOwner oldowner = CurrentResourceOwner;
3209+
MemoryContext work_cxt;
3210+
bool found;
3211+
int i;
32103212

32113213
check_spi_usage_allowed();
32123214

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

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

32483273
qdesc->argtypes[i] = typId;
3249-
perm_fmgr_info(typInput, &(qdesc->arginfuncs[i]));
3274+
fmgr_info_cxt(typInput, &(qdesc->arginfuncs[i]), plan_cxt);
32503275
qdesc->argtypioparams[i] = typIOParam;
32513276
}
32523277

@@ -3274,6 +3299,17 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
32743299
/* Release the procCxt copy to avoid within-function memory leak */
32753300
SPI_freeplan(plan);
32763301

3302+
/************************************************************
3303+
* Insert a hashtable entry for the plan.
3304+
************************************************************/
3305+
hash_entry = hash_search(plperl_active_interp->query_hash,
3306+
qdesc->qname,
3307+
HASH_ENTER, &found);
3308+
hash_entry->query_data = qdesc;
3309+
3310+
/* Get rid of workspace */
3311+
MemoryContextDelete(work_cxt);
3312+
32773313
/* Commit the inner transaction, return to outer xact context */
32783314
ReleaseCurrentSubTransaction();
32793315
MemoryContextSwitchTo(oldcontext);
@@ -3289,16 +3325,21 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
32893325
{
32903326
ErrorData *edata;
32913327

3292-
free(qdesc->argtypes);
3293-
free(qdesc->arginfuncs);
3294-
free(qdesc->argtypioparams);
3295-
free(qdesc);
3296-
32973328
/* Save error info */
32983329
MemoryContextSwitchTo(oldcontext);
32993330
edata = CopyErrorData();
33003331
FlushErrorState();
33013332

3333+
/* Drop anything we managed to allocate */
3334+
if (hash_entry)
3335+
hash_search(plperl_active_interp->query_hash,
3336+
qdesc->qname,
3337+
HASH_REMOVE, NULL);
3338+
if (plan_cxt)
3339+
MemoryContextDelete(plan_cxt);
3340+
if (plan)
3341+
SPI_freeplan(plan);
3342+
33023343
/* Abort the inner transaction */
33033344
RollbackAndReleaseCurrentSubTransaction();
33043345
MemoryContextSwitchTo(oldcontext);
@@ -3320,14 +3361,8 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
33203361
PG_END_TRY();
33213362

33223363
/************************************************************
3323-
* Insert a hashtable entry for the plan and return
3324-
* the key to the caller.
3364+
* Return the query's hash key to the caller.
33253365
************************************************************/
3326-
3327-
hash_entry = hash_search(plperl_active_interp->query_hash, qdesc->qname,
3328-
HASH_ENTER, &found);
3329-
hash_entry->query_data = qdesc;
3330-
33313366
return cstr2sv(qdesc->qname);
33323367
}
33333368

@@ -3362,16 +3397,14 @@ plperl_spi_exec_prepared(char *query, HV *attr, int argc, SV **argv)
33623397
/************************************************************
33633398
* Fetch the saved plan descriptor, see if it's o.k.
33643399
************************************************************/
3365-
33663400
hash_entry = hash_search(plperl_active_interp->query_hash, query,
33673401
HASH_FIND, NULL);
33683402
if (hash_entry == NULL)
33693403
elog(ERROR, "spi_exec_prepared: Invalid prepared query passed");
33703404

33713405
qdesc = hash_entry->query_data;
3372-
33733406
if (qdesc == NULL)
3374-
elog(ERROR, "spi_exec_prepared: panic - plperl query_hash value vanished");
3407+
elog(ERROR, "spi_exec_prepared: plperl query_hash value vanished");
33753408

33763409
if (qdesc->nargs != argc)
33773410
elog(ERROR, "spi_exec_prepared: expected %d argument(s), %d passed",
@@ -3503,12 +3536,11 @@ plperl_spi_query_prepared(char *query, int argc, SV **argv)
35033536
hash_entry = hash_search(plperl_active_interp->query_hash, query,
35043537
HASH_FIND, NULL);
35053538
if (hash_entry == NULL)
3506-
elog(ERROR, "spi_exec_prepared: Invalid prepared query passed");
3539+
elog(ERROR, "spi_query_prepared: Invalid prepared query passed");
35073540

35083541
qdesc = hash_entry->query_data;
3509-
35103542
if (qdesc == NULL)
3511-
elog(ERROR, "spi_query_prepared: panic - plperl query_hash value vanished");
3543+
elog(ERROR, "spi_query_prepared: plperl query_hash value vanished");
35123544

35133545
if (qdesc->nargs != argc)
35143546
elog(ERROR, "spi_query_prepared: expected %d argument(s), %d passed",
@@ -3613,12 +3645,12 @@ plperl_spi_freeplan(char *query)
36133645
hash_entry = hash_search(plperl_active_interp->query_hash, query,
36143646
HASH_FIND, NULL);
36153647
if (hash_entry == NULL)
3616-
elog(ERROR, "spi_exec_prepared: Invalid prepared query passed");
3648+
elog(ERROR, "spi_freeplan: Invalid prepared query passed");
36173649

36183650
qdesc = hash_entry->query_data;
3619-
36203651
if (qdesc == NULL)
3621-
elog(ERROR, "spi_exec_freeplan: panic - plperl query_hash value vanished");
3652+
elog(ERROR, "spi_freeplan: plperl query_hash value vanished");
3653+
plan = qdesc->plan;
36223654

36233655
/*
36243656
* free all memory before SPI_freeplan, so if it dies, nothing will be
@@ -3627,11 +3659,7 @@ plperl_spi_freeplan(char *query)
36273659
hash_search(plperl_active_interp->query_hash, query,
36283660
HASH_REMOVE, NULL);
36293661

3630-
plan = qdesc->plan;
3631-
free(qdesc->argtypes);
3632-
free(qdesc->arginfuncs);
3633-
free(qdesc->argtypioparams);
3634-
free(qdesc);
3662+
MemoryContextDelete(qdesc->plan_cxt);
36353663

36363664
SPI_freeplan(plan);
36373665
}

0 commit comments

Comments
 (0)