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

Commit2ba890c

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 parent34e09e7 commit2ba890c

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
/*
@@ -3985,6 +3995,17 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
39853995
estate->paramLI->parserSetupArg=NULL;/* filled during use */
39863996
estate->paramLI->numParams=estate->ndatums;
39873997

3998+
/* Create the session-wide cast-expression hash if we didn't already */
3999+
if (cast_expr_hash==NULL)
4000+
{
4001+
ctl.keysize=sizeof(plpgsql_CastHashKey);
4002+
ctl.entrysize=sizeof(plpgsql_CastExprHashEntry);
4003+
cast_expr_hash=hash_create("PLpgSQL cast expressions",
4004+
16,/* start small and extend */
4005+
&ctl,
4006+
HASH_ELEM |HASH_BLOBS);
4007+
}
4008+
39884009
/* set up for use of appropriate simple-expression EState and cast hash */
39894010
if (simple_eval_estate)
39904011
{
@@ -3997,27 +4018,21 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
39974018
16,/* start small and extend */
39984019
&ctl,
39994020
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
4000-
estate->cast_hash_context=CurrentMemoryContext;
40014021
}
40024022
else
40034023
{
40044024
estate->simple_eval_estate=shared_simple_eval_estate;
40054025
/* Create the session-wide cast-info hash table if we didn't already */
40064026
if (shared_cast_hash==NULL)
40074027
{
4008-
shared_cast_context=AllocSetContextCreate(TopMemoryContext,
4009-
"PLpgSQL cast info",
4010-
ALLOCSET_DEFAULT_SIZES);
40114028
ctl.keysize=sizeof(plpgsql_CastHashKey);
40124029
ctl.entrysize=sizeof(plpgsql_CastHashEntry);
4013-
ctl.hcxt=shared_cast_context;
40144030
shared_cast_hash=hash_create("PLpgSQL cast cache",
40154031
16,/* start small and extend */
40164032
&ctl,
4017-
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
4033+
HASH_ELEM |HASH_BLOBS);
40184034
}
40194035
estate->cast_hash=shared_cast_hash;
4020-
estate->cast_hash_context=shared_cast_context;
40214036
}
40224037
/* likewise for the simple-expression resource owner */
40234038
if (simple_eval_resowner)
@@ -7715,6 +7730,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
77157730
{
77167731
plpgsql_CastHashKeycast_key;
77177732
plpgsql_CastHashEntry*cast_entry;
7733+
plpgsql_CastExprHashEntry*expr_entry;
77187734
boolfound;
77197735
LocalTransactionIdcurlxid;
77207736
MemoryContextoldcontext;
@@ -7728,10 +7744,28 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
77287744
(void*)&cast_key,
77297745
HASH_ENTER,&found);
77307746
if (!found)/* initialize if new entry */
7731-
cast_entry->cast_cexpr=NULL;
7747+
{
7748+
/* We need a second lookup to see if a cast_expr_hash entry exists */
7749+
expr_entry= (plpgsql_CastExprHashEntry*)hash_search(cast_expr_hash,
7750+
&cast_key,
7751+
HASH_ENTER,
7752+
&found);
7753+
if (!found)/* initialize if new expr entry */
7754+
expr_entry->cast_cexpr=NULL;
77327755

7733-
if (cast_entry->cast_cexpr==NULL||
7734-
!cast_entry->cast_cexpr->is_valid)
7756+
cast_entry->cast_centry=expr_entry;
7757+
cast_entry->cast_exprstate=NULL;
7758+
cast_entry->cast_in_use= false;
7759+
cast_entry->cast_lxid=InvalidLocalTransactionId;
7760+
}
7761+
else
7762+
{
7763+
/* Use always-valid link to avoid a second hash lookup */
7764+
expr_entry=cast_entry->cast_centry;
7765+
}
7766+
7767+
if (expr_entry->cast_cexpr==NULL||
7768+
!expr_entry->cast_cexpr->is_valid)
77357769
{
77367770
/*
77377771
* We've not looked up this coercion before, or we have but the cached
@@ -7744,10 +7778,10 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
77447778
/*
77457779
* Drop old cached expression if there is one.
77467780
*/
7747-
if (cast_entry->cast_cexpr)
7781+
if (expr_entry->cast_cexpr)
77487782
{
7749-
FreeCachedExpression(cast_entry->cast_cexpr);
7750-
cast_entry->cast_cexpr=NULL;
7783+
FreeCachedExpression(expr_entry->cast_cexpr);
7784+
expr_entry->cast_cexpr=NULL;
77517785
}
77527786

77537787
/*
@@ -7828,9 +7862,11 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
78287862
((RelabelType*)cast_expr)->arg== (Expr*)placeholder)
78297863
cast_expr=NULL;
78307864

7831-
/* Now we can fill in the hashtable entry. */
7832-
cast_entry->cast_cexpr=cast_cexpr;
7833-
cast_entry->cast_expr= (Expr*)cast_expr;
7865+
/* Now we can fill in the expression hashtable entry. */
7866+
expr_entry->cast_cexpr=cast_cexpr;
7867+
expr_entry->cast_expr= (Expr*)cast_expr;
7868+
7869+
/* Be sure to reset the exprstate hashtable entry, too. */
78347870
cast_entry->cast_exprstate=NULL;
78357871
cast_entry->cast_in_use= false;
78367872
cast_entry->cast_lxid=InvalidLocalTransactionId;
@@ -7839,7 +7875,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
78397875
}
78407876

78417877
/* Done if we have determined that this is a no-op cast. */
7842-
if (cast_entry->cast_expr==NULL)
7878+
if (expr_entry->cast_expr==NULL)
78437879
returnNULL;
78447880

78457881
/*
@@ -7858,7 +7894,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
78587894
if (cast_entry->cast_lxid!=curlxid||cast_entry->cast_in_use)
78597895
{
78607896
oldcontext=MemoryContextSwitchTo(estate->simple_eval_estate->es_query_cxt);
7861-
cast_entry->cast_exprstate=ExecInitExpr(cast_entry->cast_expr,NULL);
7897+
cast_entry->cast_exprstate=ExecInitExpr(expr_entry->cast_expr,NULL);
78627898
cast_entry->cast_in_use= false;
78637899
cast_entry->cast_lxid=curlxid;
78647900
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