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

Commitee71cad

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 parent082e571 commitee71cad

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
@@ -134,18 +134,22 @@ static SimpleEcontextStackEntry *simple_econtext_stack = NULL;
134134
MemoryContextAllocZero(get_eval_mcontext(estate), sz)
135135

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

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

170-
staticMemoryContextshared_cast_context=NULL;
180+
staticHTAB*cast_expr_hash=NULL;
171181
staticHTAB*shared_cast_hash=NULL;
172182

173183
/*
@@ -3966,6 +3976,18 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
39663976
estate->paramLI->parserSetupArg=NULL;/* filled during use */
39673977
estate->paramLI->numParams=estate->ndatums;
39683978

3979+
/* Create the session-wide cast-expression hash if we didn't already */
3980+
if (cast_expr_hash==NULL)
3981+
{
3982+
memset(&ctl,0,sizeof(ctl));
3983+
ctl.keysize=sizeof(plpgsql_CastHashKey);
3984+
ctl.entrysize=sizeof(plpgsql_CastExprHashEntry);
3985+
cast_expr_hash=hash_create("PLpgSQL cast expressions",
3986+
16,/* start small and extend */
3987+
&ctl,
3988+
HASH_ELEM |HASH_BLOBS);
3989+
}
3990+
39693991
/* set up for use of appropriate simple-expression EState and cast hash */
39703992
if (simple_eval_estate)
39713993
{
@@ -3979,28 +4001,22 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
39794001
16,/* start small and extend */
39804002
&ctl,
39814003
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
3982-
estate->cast_hash_context=CurrentMemoryContext;
39834004
}
39844005
else
39854006
{
39864007
estate->simple_eval_estate=shared_simple_eval_estate;
39874008
/* Create the session-wide cast-info hash table if we didn't already */
39884009
if (shared_cast_hash==NULL)
39894010
{
3990-
shared_cast_context=AllocSetContextCreate(TopMemoryContext,
3991-
"PLpgSQL cast info",
3992-
ALLOCSET_DEFAULT_SIZES);
39934011
memset(&ctl,0,sizeof(ctl));
39944012
ctl.keysize=sizeof(plpgsql_CastHashKey);
39954013
ctl.entrysize=sizeof(plpgsql_CastHashEntry);
3996-
ctl.hcxt=shared_cast_context;
39974014
shared_cast_hash=hash_create("PLpgSQL cast cache",
39984015
16,/* start small and extend */
39994016
&ctl,
4000-
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
4017+
HASH_ELEM |HASH_BLOBS);
40014018
}
40024019
estate->cast_hash=shared_cast_hash;
4003-
estate->cast_hash_context=shared_cast_context;
40044020
}
40054021

40064022
/*
@@ -7825,6 +7841,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
78257841
{
78267842
plpgsql_CastHashKeycast_key;
78277843
plpgsql_CastHashEntry*cast_entry;
7844+
plpgsql_CastExprHashEntry*expr_entry;
78287845
boolfound;
78297846
LocalTransactionIdcurlxid;
78307847
MemoryContextoldcontext;
@@ -7838,10 +7855,28 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
78387855
(void*)&cast_key,
78397856
HASH_ENTER,&found);
78407857
if (!found)/* initialize if new entry */
7841-
cast_entry->cast_cexpr=NULL;
7858+
{
7859+
/* We need a second lookup to see if a cast_expr_hash entry exists */
7860+
expr_entry= (plpgsql_CastExprHashEntry*)hash_search(cast_expr_hash,
7861+
&cast_key,
7862+
HASH_ENTER,
7863+
&found);
7864+
if (!found)/* initialize if new expr entry */
7865+
expr_entry->cast_cexpr=NULL;
7866+
7867+
cast_entry->cast_centry=expr_entry;
7868+
cast_entry->cast_exprstate=NULL;
7869+
cast_entry->cast_in_use= false;
7870+
cast_entry->cast_lxid=InvalidLocalTransactionId;
7871+
}
7872+
else
7873+
{
7874+
/* Use always-valid link to avoid a second hash lookup */
7875+
expr_entry=cast_entry->cast_centry;
7876+
}
78427877

7843-
if (cast_entry->cast_cexpr==NULL||
7844-
!cast_entry->cast_cexpr->is_valid)
7878+
if (expr_entry->cast_cexpr==NULL||
7879+
!expr_entry->cast_cexpr->is_valid)
78457880
{
78467881
/*
78477882
* We've not looked up this coercion before, or we have but the cached
@@ -7854,10 +7889,10 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
78547889
/*
78557890
* Drop old cached expression if there is one.
78567891
*/
7857-
if (cast_entry->cast_cexpr)
7892+
if (expr_entry->cast_cexpr)
78587893
{
7859-
FreeCachedExpression(cast_entry->cast_cexpr);
7860-
cast_entry->cast_cexpr=NULL;
7894+
FreeCachedExpression(expr_entry->cast_cexpr);
7895+
expr_entry->cast_cexpr=NULL;
78617896
}
78627897

78637898
/*
@@ -7933,9 +7968,11 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
79337968
((RelabelType*)cast_expr)->arg== (Expr*)placeholder)
79347969
cast_expr=NULL;
79357970

7936-
/* Now we can fill in the hashtable entry. */
7937-
cast_entry->cast_cexpr=cast_cexpr;
7938-
cast_entry->cast_expr= (Expr*)cast_expr;
7971+
/* Now we can fill in the expression hashtable entry. */
7972+
expr_entry->cast_cexpr=cast_cexpr;
7973+
expr_entry->cast_expr= (Expr*)cast_expr;
7974+
7975+
/* Be sure to reset the exprstate hashtable entry, too. */
79397976
cast_entry->cast_exprstate=NULL;
79407977
cast_entry->cast_in_use= false;
79417978
cast_entry->cast_lxid=InvalidLocalTransactionId;
@@ -7944,7 +7981,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
79447981
}
79457982

79467983
/* Done if we have determined that this is a no-op cast. */
7947-
if (cast_entry->cast_expr==NULL)
7984+
if (expr_entry->cast_expr==NULL)
79487985
returnNULL;
79497986

79507987
/*
@@ -7963,7 +8000,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
79638000
if (cast_entry->cast_lxid!=curlxid||cast_entry->cast_in_use)
79648001
{
79658002
oldcontext=MemoryContextSwitchTo(estate->simple_eval_estate->es_query_cxt);
7966-
cast_entry->cast_exprstate=ExecInitExpr(cast_entry->cast_expr,NULL);
8003+
cast_entry->cast_exprstate=ExecInitExpr(expr_entry->cast_expr,NULL);
79678004
cast_entry->cast_in_use= false;
79688005
cast_entry->cast_lxid=curlxid;
79698006
MemoryContextSwitchTo(oldcontext);

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

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

10871087
/* lookup table to use for executing type casts */
10881088
HTAB*cast_hash;
1089-
MemoryContextcast_hash_context;
1089+
MemoryContextcast_hash_context;/* not used; now always NULL */
10901090

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

0 commit comments

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