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

Commit35b039a

Browse files
committed
Repair oversights in the mechanism used to store compiled plpgsql functions.
The original coding failed (tried to access deallocated memory) if there weretwo active call sites (fn_extra pointers) for the same function and thefunction definition was updated. Also, if an update of a recursive functionwas detected upon nested entry to the function, the existing compiled versionwas summarily deallocated, resulting in crash upon return to the outerinstance. Problem observed while studying a bug report from SergiyVyshnevetskiy.Bug does not exist before 8.1 since older versions just leaked the memory ofobsoleted compiled functions, rather than trying to reclaim it.
1 parent33d78c9 commit35b039a

File tree

3 files changed

+115
-31
lines changed

3 files changed

+115
-31
lines changed

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

Lines changed: 88 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_comp.c,v 1.109 2007/01/05 22:20:02 momjian Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_comp.c,v 1.110 2007/01/30 22:05:12 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -93,6 +93,7 @@ static const ExceptionLabelMap exception_label_map[] = {
9393
*/
9494
staticPLpgSQL_function*do_compile(FunctionCallInfofcinfo,
9595
HeapTupleprocTup,
96+
PLpgSQL_function*function,
9697
PLpgSQL_func_hashkey*hashkey,
9798
boolforValidator);
9899
staticPLpgSQL_row*build_row_from_class(OidclassOid);
@@ -130,6 +131,7 @@ plpgsql_compile(FunctionCallInfo fcinfo, bool forValidator)
130131
Form_pg_procprocStruct;
131132
PLpgSQL_function*function;
132133
PLpgSQL_func_hashkeyhashkey;
134+
boolfunction_valid= false;
133135
boolhashkey_valid= false;
134136

135137
/*
@@ -148,6 +150,7 @@ plpgsql_compile(FunctionCallInfo fcinfo, bool forValidator)
148150
*/
149151
function= (PLpgSQL_function*)fcinfo->flinfo->fn_extra;
150152

153+
recheck:
151154
if (!function)
152155
{
153156
/* Compute hashkey using function signature and actual arg types */
@@ -161,19 +164,43 @@ plpgsql_compile(FunctionCallInfo fcinfo, bool forValidator)
161164
if (function)
162165
{
163166
/* We have a compiled function, but is it still valid? */
164-
if (!(function->fn_xmin==HeapTupleHeaderGetXmin(procTup->t_data)&&
165-
function->fn_cmin==HeapTupleHeaderGetCmin(procTup->t_data)))
167+
if (function->fn_xmin==HeapTupleHeaderGetXmin(procTup->t_data)&&
168+
function->fn_cmin==HeapTupleHeaderGetCmin(procTup->t_data))
169+
function_valid= true;
170+
else
166171
{
167-
/* Nope, drop the function and associated storage */
172+
/*
173+
* Nope, so remove it from hashtable and try to drop associated
174+
* storage (if not done already).
175+
*/
168176
delete_function(function);
169-
function=NULL;
177+
/*
178+
* If the function isn't in active use then we can overwrite the
179+
* func struct with new data, allowing any other existing fn_extra
180+
* pointers to make use of the new definition on their next use.
181+
* If it is in use then just leave it alone and make a new one.
182+
* (The active invocations will run to completion using the
183+
* previous definition, and then the cache entry will just be
184+
* leaked; doesn't seem worth adding code to clean it up, given
185+
* what a corner case this is.)
186+
*
187+
* If we found the function struct via fn_extra then it's possible
188+
* a replacement has already been made, so go back and recheck
189+
* the hashtable.
190+
*/
191+
if (function->use_count!=0)
192+
{
193+
function=NULL;
194+
if (!hashkey_valid)
195+
gotorecheck;
196+
}
170197
}
171198
}
172199

173200
/*
174201
* If the function wasn't found or was out-of-date, we have to compile it
175202
*/
176-
if (!function)
203+
if (!function_valid)
177204
{
178205
/*
179206
* Calculate hashkey if we didn't already; we'll need it to store the
@@ -186,7 +213,8 @@ plpgsql_compile(FunctionCallInfo fcinfo, bool forValidator)
186213
/*
187214
* Do the hard part.
188215
*/
189-
function=do_compile(fcinfo,procTup,&hashkey,forValidator);
216+
function=do_compile(fcinfo,procTup,function,
217+
&hashkey,forValidator);
190218
}
191219

192220
ReleaseSysCache(procTup);
@@ -205,6 +233,9 @@ plpgsql_compile(FunctionCallInfo fcinfo, bool forValidator)
205233
/*
206234
* This is the slow part of plpgsql_compile().
207235
*
236+
* The passed-in "function" pointer is either NULL or an already-allocated
237+
* function struct to overwrite.
238+
*
208239
* While compiling a function, the CurrentMemoryContext is the
209240
* per-function memory context of the function we are compiling. That
210241
* means a palloc() will allocate storage with the same lifetime as
@@ -217,16 +248,19 @@ plpgsql_compile(FunctionCallInfo fcinfo, bool forValidator)
217248
* switch into a short-term context before calling into the
218249
* backend. An appropriate context for performing short-term
219250
* allocations is the compile_tmp_cxt.
251+
*
252+
* NB: this code is not re-entrant. We assume that nothing we do here could
253+
* result in the invocation of another plpgsql function.
220254
*/
221255
staticPLpgSQL_function*
222256
do_compile(FunctionCallInfofcinfo,
223257
HeapTupleprocTup,
258+
PLpgSQL_function*function,
224259
PLpgSQL_func_hashkey*hashkey,
225260
boolforValidator)
226261
{
227262
Form_pg_procprocStruct= (Form_pg_proc)GETSTRUCT(procTup);
228263
intfunctype=CALLED_AS_TRIGGER(fcinfo) ?T_TRIGGER :T_FUNCTION;
229-
PLpgSQL_function*function;
230264
Datumprosrcdatum;
231265
boolisnull;
232266
char*proc_source;
@@ -292,18 +326,31 @@ do_compile(FunctionCallInfo fcinfo,
292326
plpgsql_check_syntax=forValidator;
293327

294328
/*
295-
* Create the new function node. We allocate the function and all of its
296-
* compile-time storage (e.g. parse tree) in its own memory context. This
297-
* allows us to reclaim the function's storage cleanly.
329+
* Create the new function struct, if not done already. The function
330+
* structs are never thrown away, so keep them in TopMemoryContext.
331+
*/
332+
if (function==NULL)
333+
{
334+
function= (PLpgSQL_function*)
335+
MemoryContextAllocZero(TopMemoryContext,sizeof(PLpgSQL_function));
336+
}
337+
else
338+
{
339+
/* re-using a previously existing struct, so clear it out */
340+
memset(function,0,sizeof(PLpgSQL_function));
341+
}
342+
plpgsql_curr_compile=function;
343+
344+
/*
345+
* All the rest of the compile-time storage (e.g. parse tree) is kept in
346+
* its own memory context, so it can be reclaimed easily.
298347
*/
299348
func_cxt=AllocSetContextCreate(TopMemoryContext,
300349
"PL/PgSQL function context",
301350
ALLOCSET_DEFAULT_MINSIZE,
302351
ALLOCSET_DEFAULT_INITSIZE,
303352
ALLOCSET_DEFAULT_MAXSIZE);
304353
compile_tmp_cxt=MemoryContextSwitchTo(func_cxt);
305-
function=palloc0(sizeof(*function));
306-
plpgsql_curr_compile=function;
307354

308355
function->fn_name=pstrdup(NameStr(procStruct->proname));
309356
function->fn_oid=fcinfo->flinfo->fn_oid;
@@ -1990,19 +2037,32 @@ plpgsql_resolve_polymorphic_argtypes(int numargs,
19902037
}
19912038
}
19922039

2040+
/*
2041+
* delete_function - clean up as much as possible of a stale function cache
2042+
*
2043+
* We can't release the PLpgSQL_function struct itself, because of the
2044+
* possibility that there are fn_extra pointers to it. We can release
2045+
* the subsidiary storage, but only if there are no active evaluations
2046+
* in progress. Otherwise we'll just leak that storage. Since the
2047+
* case would only occur if a pg_proc update is detected during a nested
2048+
* recursive call on the function, a leak seems acceptable.
2049+
*
2050+
* Note that this can be called more than once if there are multiple fn_extra
2051+
* pointers to the same function cache. Hence be careful not to do things
2052+
* twice.
2053+
*/
19932054
staticvoid
19942055
delete_function(PLpgSQL_function*func)
19952056
{
1996-
/* remove function from hash table */
2057+
/* remove function from hash table(might be done already)*/
19972058
plpgsql_HashTableDelete(func);
19982059

1999-
/* release the function's storage */
2000-
MemoryContextDelete(func->fn_cxt);
2001-
2002-
/*
2003-
* Caller should be sure not to use passed-in pointer, as it now points to
2004-
* pfree'd storage
2005-
*/
2060+
/* release the function's storage if safe and not done already */
2061+
if (func->use_count==0&&func->fn_cxt)
2062+
{
2063+
MemoryContextDelete(func->fn_cxt);
2064+
func->fn_cxt=NULL;
2065+
}
20062066
}
20072067

20082068
/* exported so we can call it from plpgsql_init() */
@@ -2063,10 +2123,17 @@ plpgsql_HashTableDelete(PLpgSQL_function *function)
20632123
{
20642124
plpgsql_HashEnt*hentry;
20652125

2126+
/* do nothing if not in table */
2127+
if (function->fn_hashkey==NULL)
2128+
return;
2129+
20662130
hentry= (plpgsql_HashEnt*)hash_search(plpgsql_HashTable,
20672131
(void*)function->fn_hashkey,
20682132
HASH_REMOVE,
20692133
NULL);
20702134
if (hentry==NULL)
20712135
elog(WARNING,"trying to delete function that does not exist");
2136+
2137+
/* remove back link, which no longer points to allocated storage */
2138+
function->fn_hashkey=NULL;
20722139
}

‎src/pl/plpgsql/src/pl_handler.c

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_handler.c,v 1.35 2007/01/28 16:15:49 tgl Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_handler.c,v 1.36 2007/01/30 22:05:13 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -79,15 +79,30 @@ plpgsql_call_handler(PG_FUNCTION_ARGS)
7979
/* Find or compile the function */
8080
func=plpgsql_compile(fcinfo, false);
8181

82-
/*
83-
* Determine if called as function or trigger and call appropriate
84-
* subhandler
85-
*/
86-
if (CALLED_AS_TRIGGER(fcinfo))
87-
retval=PointerGetDatum(plpgsql_exec_trigger(func,
82+
/* Mark the function as busy, so it can't be deleted from under us */
83+
func->use_count++;
84+
85+
PG_TRY();
86+
{
87+
/*
88+
* Determine if called as function or trigger and call appropriate
89+
* subhandler
90+
*/
91+
if (CALLED_AS_TRIGGER(fcinfo))
92+
retval=PointerGetDatum(plpgsql_exec_trigger(func,
8893
(TriggerData*)fcinfo->context));
89-
else
90-
retval=plpgsql_exec_function(func,fcinfo);
94+
else
95+
retval=plpgsql_exec_function(func,fcinfo);
96+
}
97+
PG_CATCH();
98+
{
99+
/* Decrement use-count and propagate error */
100+
func->use_count--;
101+
PG_RE_THROW();
102+
}
103+
PG_END_TRY();
104+
105+
func->use_count--;
91106

92107
/*
93108
* Disconnect from SPI manager

‎src/pl/plpgsql/src/plpgsql.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.83 2007/01/28 16:15:49 tgl Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.84 2007/01/30 22:05:13 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -580,6 +580,8 @@ typedef struct PLpgSQL_function
580580
intndatums;
581581
PLpgSQL_datum**datums;
582582
PLpgSQL_stmt_block*action;
583+
584+
unsigned longuse_count;
583585
}PLpgSQL_function;
584586

585587

0 commit comments

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