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

Commitbfa6910

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 parent50b23e4 commitbfa6910

File tree

2 files changed

+67
-30
lines changed

2 files changed

+67
-30
lines changed

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

Lines changed: 66 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -144,18 +144,22 @@ static ResourceOwner shared_simple_eval_resowner = NULL;
144144
MemoryContextAllocZero(get_eval_mcontext(estate), sz)
145145

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

169-
typedefstruct/*cast_hash table entry */
173+
typedefstruct/*cast_expr_hash table entry */
170174
{
171175
plpgsql_CastHashKeykey;/* hash key --- MUST BE FIRST */
172176
Expr*cast_expr;/* cast expression, or NULL if no-op cast */
173177
CachedExpression*cast_cexpr;/* cached expression backing the above */
178+
}plpgsql_CastExprHashEntry;
179+
180+
typedefstruct/* cast_hash table entry */
181+
{
182+
plpgsql_CastHashKeykey;/* hash key --- MUST BE FIRST */
183+
plpgsql_CastExprHashEntry*cast_centry;/* link to matching expr entry */
174184
/* ExprState is valid only when cast_lxid matches current LXID */
175185
ExprState*cast_exprstate;/* expression's eval tree */
176186
boolcast_in_use;/* true while we're executing eval tree */
177187
LocalTransactionIdcast_lxid;
178188
}plpgsql_CastHashEntry;
179189

180-
staticMemoryContextshared_cast_context=NULL;
190+
staticHTAB*cast_expr_hash=NULL;
181191
staticHTAB*shared_cast_hash=NULL;
182192

183193
/*
@@ -3980,6 +3990,18 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
39803990
estate->paramLI->parserSetupArg=NULL;/* filled during use */
39813991
estate->paramLI->numParams=estate->ndatums;
39823992

3993+
/* Create the session-wide cast-expression hash if we didn't already */
3994+
if (cast_expr_hash==NULL)
3995+
{
3996+
memset(&ctl,0,sizeof(ctl));
3997+
ctl.keysize=sizeof(plpgsql_CastHashKey);
3998+
ctl.entrysize=sizeof(plpgsql_CastExprHashEntry);
3999+
cast_expr_hash=hash_create("PLpgSQL cast expressions",
4000+
16,/* start small and extend */
4001+
&ctl,
4002+
HASH_ELEM |HASH_BLOBS);
4003+
}
4004+
39834005
/* set up for use of appropriate simple-expression EState and cast hash */
39844006
if (simple_eval_estate)
39854007
{
@@ -3993,28 +4015,22 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
39934015
16,/* start small and extend */
39944016
&ctl,
39954017
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
3996-
estate->cast_hash_context=CurrentMemoryContext;
39974018
}
39984019
else
39994020
{
40004021
estate->simple_eval_estate=shared_simple_eval_estate;
40014022
/* Create the session-wide cast-info hash table if we didn't already */
40024023
if (shared_cast_hash==NULL)
40034024
{
4004-
shared_cast_context=AllocSetContextCreate(TopMemoryContext,
4005-
"PLpgSQL cast info",
4006-
ALLOCSET_DEFAULT_SIZES);
40074025
memset(&ctl,0,sizeof(ctl));
40084026
ctl.keysize=sizeof(plpgsql_CastHashKey);
40094027
ctl.entrysize=sizeof(plpgsql_CastHashEntry);
4010-
ctl.hcxt=shared_cast_context;
40114028
shared_cast_hash=hash_create("PLpgSQL cast cache",
40124029
16,/* start small and extend */
40134030
&ctl,
4014-
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
4031+
HASH_ELEM |HASH_BLOBS);
40154032
}
40164033
estate->cast_hash=shared_cast_hash;
4017-
estate->cast_hash_context=shared_cast_context;
40184034
}
40194035
/* likewise for the simple-expression resource owner */
40204036
if (simple_eval_resowner)
@@ -7916,6 +7932,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
79167932
{
79177933
plpgsql_CastHashKeycast_key;
79187934
plpgsql_CastHashEntry*cast_entry;
7935+
plpgsql_CastExprHashEntry*expr_entry;
79197936
boolfound;
79207937
LocalTransactionIdcurlxid;
79217938
MemoryContextoldcontext;
@@ -7929,10 +7946,28 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
79297946
(void*)&cast_key,
79307947
HASH_ENTER,&found);
79317948
if (!found)/* initialize if new entry */
7932-
cast_entry->cast_cexpr=NULL;
7949+
{
7950+
/* We need a second lookup to see if a cast_expr_hash entry exists */
7951+
expr_entry= (plpgsql_CastExprHashEntry*)hash_search(cast_expr_hash,
7952+
&cast_key,
7953+
HASH_ENTER,
7954+
&found);
7955+
if (!found)/* initialize if new expr entry */
7956+
expr_entry->cast_cexpr=NULL;
7957+
7958+
cast_entry->cast_centry=expr_entry;
7959+
cast_entry->cast_exprstate=NULL;
7960+
cast_entry->cast_in_use= false;
7961+
cast_entry->cast_lxid=InvalidLocalTransactionId;
7962+
}
7963+
else
7964+
{
7965+
/* Use always-valid link to avoid a second hash lookup */
7966+
expr_entry=cast_entry->cast_centry;
7967+
}
79337968

7934-
if (cast_entry->cast_cexpr==NULL||
7935-
!cast_entry->cast_cexpr->is_valid)
7969+
if (expr_entry->cast_cexpr==NULL||
7970+
!expr_entry->cast_cexpr->is_valid)
79367971
{
79377972
/*
79387973
* We've not looked up this coercion before, or we have but the cached
@@ -7945,10 +7980,10 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
79457980
/*
79467981
* Drop old cached expression if there is one.
79477982
*/
7948-
if (cast_entry->cast_cexpr)
7983+
if (expr_entry->cast_cexpr)
79497984
{
7950-
FreeCachedExpression(cast_entry->cast_cexpr);
7951-
cast_entry->cast_cexpr=NULL;
7985+
FreeCachedExpression(expr_entry->cast_cexpr);
7986+
expr_entry->cast_cexpr=NULL;
79527987
}
79537988

79547989
/*
@@ -8024,9 +8059,11 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
80248059
((RelabelType*)cast_expr)->arg== (Expr*)placeholder)
80258060
cast_expr=NULL;
80268061

8027-
/* Now we can fill in the hashtable entry. */
8028-
cast_entry->cast_cexpr=cast_cexpr;
8029-
cast_entry->cast_expr= (Expr*)cast_expr;
8062+
/* Now we can fill in the expression hashtable entry. */
8063+
expr_entry->cast_cexpr=cast_cexpr;
8064+
expr_entry->cast_expr= (Expr*)cast_expr;
8065+
8066+
/* Be sure to reset the exprstate hashtable entry, too. */
80308067
cast_entry->cast_exprstate=NULL;
80318068
cast_entry->cast_in_use= false;
80328069
cast_entry->cast_lxid=InvalidLocalTransactionId;
@@ -8035,7 +8072,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
80358072
}
80368073

80378074
/* Done if we have determined that this is a no-op cast. */
8038-
if (cast_entry->cast_expr==NULL)
8075+
if (expr_entry->cast_expr==NULL)
80398076
returnNULL;
80408077

80418078
/*
@@ -8054,7 +8091,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
80548091
if (cast_entry->cast_lxid!=curlxid||cast_entry->cast_in_use)
80558092
{
80568093
oldcontext=MemoryContextSwitchTo(estate->simple_eval_estate->es_query_cxt);
8057-
cast_entry->cast_exprstate=ExecInitExpr(cast_entry->cast_expr,NULL);
8094+
cast_entry->cast_exprstate=ExecInitExpr(expr_entry->cast_expr,NULL);
80588095
cast_entry->cast_in_use= false;
80598096
cast_entry->cast_lxid=curlxid;
80608097
MemoryContextSwitchTo(oldcontext);

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

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

10981098
/* lookup table to use for executing type casts */
10991099
HTAB*cast_hash;
1100-
MemoryContextcast_hash_context;
1100+
MemoryContextcast_hash_context;/* not used; now always NULL */
11011101

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

0 commit comments

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