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

Commitc1598d8

Browse files
committed
Fix memory leakage in plpgsql DO blocks that use cast expressions.
Commit04fe805 modified plpgsql so that datatype casts make use ofexpressions cached by plancache.c, in place of older code where theseexpression trees were managed by plpgsql itself. However, I (tgl)forgot that we use a separate, shorter-lived cast info hashtable inDO blocks. The new mechanism thus resulted in session-lifespanleakage of the plancache data once a DO block containing one or morecasts terminated. To fix, split the cast hash table into two parts,one that tracks only the plancache's CachedExpressions and one thattracks the expression state trees generated from them. DO blocks needtheir own expression state trees and hence their own version of thesecond hash table, but there's no reason they can't share theCachedExpressions with regular plpgsql functions.Per report from Ajit Awekar. Back-patch to v12 where the issuewas introduced.Ajit Awekar and Tom LaneDiscussion:https://postgr.es/m/CAHv6PyrNaqdvyWUspzd3txYQguFTBSnhx+m6tS06TnM+KWc_LQ@mail.gmail.com
1 parenta63b821 commitc1598d8

File tree

2 files changed

+66
-30
lines changed

2 files changed

+66
-30
lines changed

‎src/pl/plpgsql/src/pl_exec.c‎

Lines changed: 65 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -136,18 +136,22 @@ static ResourceOwner shared_simple_eval_resowner = NULL;
136136
MemoryContextAllocZero(get_eval_mcontext(estate), sz)
137137

138138
/*
139-
* We usea session-wide hashtable for caching cast information.
139+
* We usetwo session-wide hashtables for caching cast information.
140140
*
141-
* Once built, the compiled expression trees (cast_expr fields) survive for
142-
* the life of the session. At some point it might be worth invalidating
143-
* those after pg_cast changes, but for the moment we don't bother.
141+
* cast_expr_hash entries (of type plpgsql_CastExprHashEntry) hold compiled
142+
* expression trees for casts. These survive for the life of the session and
143+
* are shared across all PL/pgSQL functions and DO blocks. At some point it
144+
* might be worth invalidating them after pg_cast changes, but for the moment
145+
* we don't bother.
144146
*
145-
* The evaluation state trees (cast_exprstate) are managed in the same way as
146-
* simple expressions (i.e., we assume cast expressions are always simple).
147+
* There is a separate hash table shared_cast_hash (with entries of type
148+
* plpgsql_CastHashEntry) containing evaluation state trees for these
149+
* expressions, which are managed in the same way as simple expressions
150+
* (i.e., we assume cast expressions are always simple).
147151
*
148-
* As with simple expressions, DO blocks don't use theshared hashtable but
149-
* must have their own. This isn't ideal, but we don't want to deal with
150-
* multiple simple_eval_estates within a DO block.
152+
* As with simple expressions, DO blocks don't use theshared_cast_hashtable
153+
*butmust have their own evaluation state trees. This isn't ideal, but we
154+
*don't want to deal withmultiple simple_eval_estates within a DO block.
151155
*/
152156
typedefstruct/* lookup key for cast info */
153157
{
@@ -158,18 +162,24 @@ typedef struct/* lookup key for cast info */
158162
int32dsttypmod;/* destination typmod for cast */
159163
}plpgsql_CastHashKey;
160164

161-
typedefstruct/*cast_hash table entry */
165+
typedefstruct/*cast_expr_hash table entry */
162166
{
163167
plpgsql_CastHashKeykey;/* hash key --- MUST BE FIRST */
164168
Expr*cast_expr;/* cast expression, or NULL if no-op cast */
165169
CachedExpression*cast_cexpr;/* cached expression backing the above */
170+
}plpgsql_CastExprHashEntry;
171+
172+
typedefstruct/* cast_hash table entry */
173+
{
174+
plpgsql_CastHashKeykey;/* hash key --- MUST BE FIRST */
175+
plpgsql_CastExprHashEntry*cast_centry;/* link to matching expr entry */
166176
/* ExprState is valid only when cast_lxid matches current LXID */
167177
ExprState*cast_exprstate;/* expression's eval tree */
168178
boolcast_in_use;/* true while we're executing eval tree */
169179
LocalTransactionIdcast_lxid;
170180
}plpgsql_CastHashEntry;
171181

172-
staticMemoryContextshared_cast_context=NULL;
182+
staticHTAB*cast_expr_hash=NULL;
173183
staticHTAB*shared_cast_hash=NULL;
174184

175185
/*
@@ -4019,6 +4029,17 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
40194029
estate->paramLI->parserSetupArg=NULL;/* filled during use */
40204030
estate->paramLI->numParams=estate->ndatums;
40214031

4032+
/* Create the session-wide cast-expression hash if we didn't already */
4033+
if (cast_expr_hash==NULL)
4034+
{
4035+
ctl.keysize=sizeof(plpgsql_CastHashKey);
4036+
ctl.entrysize=sizeof(plpgsql_CastExprHashEntry);
4037+
cast_expr_hash=hash_create("PLpgSQL cast expressions",
4038+
16,/* start small and extend */
4039+
&ctl,
4040+
HASH_ELEM |HASH_BLOBS);
4041+
}
4042+
40224043
/* set up for use of appropriate simple-expression EState and cast hash */
40234044
if (simple_eval_estate)
40244045
{
@@ -4031,27 +4052,21 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
40314052
16,/* start small and extend */
40324053
&ctl,
40334054
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
4034-
estate->cast_hash_context=CurrentMemoryContext;
40354055
}
40364056
else
40374057
{
40384058
estate->simple_eval_estate=shared_simple_eval_estate;
40394059
/* Create the session-wide cast-info hash table if we didn't already */
40404060
if (shared_cast_hash==NULL)
40414061
{
4042-
shared_cast_context=AllocSetContextCreate(TopMemoryContext,
4043-
"PLpgSQL cast info",
4044-
ALLOCSET_DEFAULT_SIZES);
40454062
ctl.keysize=sizeof(plpgsql_CastHashKey);
40464063
ctl.entrysize=sizeof(plpgsql_CastHashEntry);
4047-
ctl.hcxt=shared_cast_context;
40484064
shared_cast_hash=hash_create("PLpgSQL cast cache",
40494065
16,/* start small and extend */
40504066
&ctl,
4051-
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
4067+
HASH_ELEM |HASH_BLOBS);
40524068
}
40534069
estate->cast_hash=shared_cast_hash;
4054-
estate->cast_hash_context=shared_cast_context;
40554070
}
40564071
/* likewise for the simple-expression resource owner */
40574072
if (simple_eval_resowner)
@@ -7765,6 +7780,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
77657780
{
77667781
plpgsql_CastHashKeycast_key;
77677782
plpgsql_CastHashEntry*cast_entry;
7783+
plpgsql_CastExprHashEntry*expr_entry;
77687784
boolfound;
77697785
LocalTransactionIdcurlxid;
77707786
MemoryContextoldcontext;
@@ -7778,10 +7794,28 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
77787794
(void*)&cast_key,
77797795
HASH_ENTER,&found);
77807796
if (!found)/* initialize if new entry */
7781-
cast_entry->cast_cexpr=NULL;
7797+
{
7798+
/* We need a second lookup to see if a cast_expr_hash entry exists */
7799+
expr_entry= (plpgsql_CastExprHashEntry*)hash_search(cast_expr_hash,
7800+
&cast_key,
7801+
HASH_ENTER,
7802+
&found);
7803+
if (!found)/* initialize if new expr entry */
7804+
expr_entry->cast_cexpr=NULL;
77827805

7783-
if (cast_entry->cast_cexpr==NULL||
7784-
!cast_entry->cast_cexpr->is_valid)
7806+
cast_entry->cast_centry=expr_entry;
7807+
cast_entry->cast_exprstate=NULL;
7808+
cast_entry->cast_in_use= false;
7809+
cast_entry->cast_lxid=InvalidLocalTransactionId;
7810+
}
7811+
else
7812+
{
7813+
/* Use always-valid link to avoid a second hash lookup */
7814+
expr_entry=cast_entry->cast_centry;
7815+
}
7816+
7817+
if (expr_entry->cast_cexpr==NULL||
7818+
!expr_entry->cast_cexpr->is_valid)
77857819
{
77867820
/*
77877821
* We've not looked up this coercion before, or we have but the cached
@@ -7794,10 +7828,10 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
77947828
/*
77957829
* Drop old cached expression if there is one.
77967830
*/
7797-
if (cast_entry->cast_cexpr)
7831+
if (expr_entry->cast_cexpr)
77987832
{
7799-
FreeCachedExpression(cast_entry->cast_cexpr);
7800-
cast_entry->cast_cexpr=NULL;
7833+
FreeCachedExpression(expr_entry->cast_cexpr);
7834+
expr_entry->cast_cexpr=NULL;
78017835
}
78027836

78037837
/*
@@ -7878,9 +7912,11 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
78787912
((RelabelType*)cast_expr)->arg== (Expr*)placeholder)
78797913
cast_expr=NULL;
78807914

7881-
/* Now we can fill in the hashtable entry. */
7882-
cast_entry->cast_cexpr=cast_cexpr;
7883-
cast_entry->cast_expr= (Expr*)cast_expr;
7915+
/* Now we can fill in the expression hashtable entry. */
7916+
expr_entry->cast_cexpr=cast_cexpr;
7917+
expr_entry->cast_expr= (Expr*)cast_expr;
7918+
7919+
/* Be sure to reset the exprstate hashtable entry, too. */
78847920
cast_entry->cast_exprstate=NULL;
78857921
cast_entry->cast_in_use= false;
78867922
cast_entry->cast_lxid=InvalidLocalTransactionId;
@@ -7889,7 +7925,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
78897925
}
78907926

78917927
/* Done if we have determined that this is a no-op cast. */
7892-
if (cast_entry->cast_expr==NULL)
7928+
if (expr_entry->cast_expr==NULL)
78937929
returnNULL;
78947930

78957931
/*
@@ -7908,7 +7944,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
79087944
if (cast_entry->cast_lxid!=curlxid||cast_entry->cast_in_use)
79097945
{
79107946
oldcontext=MemoryContextSwitchTo(estate->simple_eval_estate->es_query_cxt);
7911-
cast_entry->cast_exprstate=ExecInitExpr(cast_entry->cast_expr,NULL);
7947+
cast_entry->cast_exprstate=ExecInitExpr(expr_entry->cast_expr,NULL);
79127948
cast_entry->cast_in_use= false;
79137949
cast_entry->cast_lxid=curlxid;
79147950
MemoryContextSwitchTo(oldcontext);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1075,7 +1075,7 @@ typedef struct PLpgSQL_execstate
10751075

10761076
/* lookup table to use for executing type casts */
10771077
HTAB*cast_hash;
1078-
MemoryContextcast_hash_context;
1078+
MemoryContextcast_hash_context;/* not used; now always NULL */
10791079

10801080
/* memory context for statement-lifespan temporary values */
10811081
MemoryContextstmt_mcontext;/* current stmt context, or NULL if none */

0 commit comments

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