Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitbe86ca1

Browse files
committed
Fix memory leakage when function compilation fails.
In pl_comp.c, initially create the plpgsql function's cache contextunder the assumed-short-lived caller's context, and reparent it underCacheMemoryContext only upon success. This avoids a process-lifespanleak of 8kB or more if the function contains syntax errors. (Thisleakage has existed for a long time without many complaints, but aswe move towards a possibly multi-threaded future, getting rid ofprocess-lifespan leaks grows more important.)In funccache.c, arrange to reclaim the CachedFunction struct in casethe language-specific compile callback function throws an error;previously, that resulted in an independent process-lifespan leak.This is arguably a new bug in v18, since the leakage now occurredfor SQL-language functions as well as plpgsql.Also, don't fill fn_xmin/fn_tid/dcallback until after successfulcompletion of the compile callback. This avoids a scenario where apartially-built function cache might appear already valid upon laterinspection, and another scenario where dcallback might fail upon beingpresented with an incomplete cache entry. We would have to reach sucha faulty cache entry via a pre-existing fn_extra pointer, so I'm notsure these scenarios correspond to any live bug. (The predecessorcode in pl_comp.c never took any care about this, and we've heard nocomplaints about that.) Still, it's better to be careful.Given the lack of field complaints, I'm not very excited aboutback-patching any of this; but it seems still in-scope for v18.Discussion:https://postgr.es/m/999171.1748300004@sss.pgh.pa.us
1 parentc861092 commitbe86ca1

File tree

2 files changed

+41
-9
lines changed

2 files changed

+41
-9
lines changed

‎src/backend/utils/cache/funccache.c

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@ cached_function_compile(FunctionCallInfo fcinfo,
491491
CachedFunctionHashKeyhashkey;
492492
boolfunction_valid= false;
493493
boolhashkey_valid= false;
494+
boolnew_function= false;
494495

495496
/*
496497
* Lookup the pg_proc tuple by Oid; we'll need it in any case
@@ -570,13 +571,15 @@ cached_function_compile(FunctionCallInfo fcinfo,
570571

571572
/*
572573
* Create the new function struct, if not done already. The function
573-
* structs are never thrown away, so keep them in TopMemoryContext.
574+
* cache entry will be kept for the life of the backend, so put it in
575+
* TopMemoryContext.
574576
*/
575577
Assert(cacheEntrySize >=sizeof(CachedFunction));
576578
if (function==NULL)
577579
{
578580
function= (CachedFunction*)
579581
MemoryContextAllocZero(TopMemoryContext,cacheEntrySize);
582+
new_function= true;
580583
}
581584
else
582585
{
@@ -585,17 +588,36 @@ cached_function_compile(FunctionCallInfo fcinfo,
585588
}
586589

587590
/*
588-
* Fill in the CachedFunction part. fn_hashkey and use_count remain
589-
* zeroes for now.
591+
* However, if function compilation fails, we'd like not to leak the
592+
* function struct, so use a PG_TRY block to prevent that. (It's up
593+
* to the compile callback function to avoid its own internal leakage
594+
* in such cases.) Unfortunately, freeing the struct is only safe if
595+
* we just allocated it: otherwise there are probably fn_extra
596+
* pointers to it.
590597
*/
591-
function->fn_xmin=HeapTupleHeaderGetRawXmin(procTup->t_data);
592-
function->fn_tid=procTup->t_self;
593-
function->dcallback=dcallback;
598+
PG_TRY();
599+
{
600+
/*
601+
* Do the hard, language-specific part.
602+
*/
603+
ccallback(fcinfo,procTup,&hashkey,function,forValidator);
604+
}
605+
PG_CATCH();
606+
{
607+
if (new_function)
608+
pfree(function);
609+
PG_RE_THROW();
610+
}
611+
PG_END_TRY();
594612

595613
/*
596-
* Do the hard, language-specific part.
614+
* Fill in the CachedFunction part. (We do this last to prevent the
615+
* function from looking valid before it's fully built.) fn_hashkey
616+
* will be set by cfunc_hashtable_insert; use_count remains zero.
597617
*/
598-
ccallback(fcinfo,procTup,&hashkey,function,forValidator);
618+
function->fn_xmin=HeapTupleHeaderGetRawXmin(procTup->t_data);
619+
function->fn_tid=procTup->t_self;
620+
function->dcallback=dcallback;
599621

600622
/*
601623
* Add the completed struct to the hash table.

‎src/pl/plpgsql/src/pl_comp.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,13 @@ plpgsql_compile_callback(FunctionCallInfo fcinfo,
226226
/*
227227
* All the permanent output of compilation (e.g. parse tree) is kept in a
228228
* per-function memory context, so it can be reclaimed easily.
229+
*
230+
* While the func_cxt needs to be long-lived, we initially make it a child
231+
* of the assumed-short-lived caller's context, and reparent it under
232+
* CacheMemoryContext only upon success. This arrangement avoids memory
233+
* leakage during compilation of a faulty function.
229234
*/
230-
func_cxt=AllocSetContextCreate(TopMemoryContext,
235+
func_cxt=AllocSetContextCreate(CurrentMemoryContext,
231236
"PL/pgSQL function",
232237
ALLOCSET_DEFAULT_SIZES);
233238
plpgsql_compile_tmp_cxt=MemoryContextSwitchTo(func_cxt);
@@ -703,6 +708,11 @@ plpgsql_compile_callback(FunctionCallInfo fcinfo,
703708
if (plpgsql_DumpExecTree)
704709
plpgsql_dumptree(function);
705710

711+
/*
712+
* All is well, so make the func_cxt long-lived
713+
*/
714+
MemoryContextSetParent(func_cxt,CacheMemoryContext);
715+
706716
/*
707717
* Pop the error context stack
708718
*/

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp