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

Commit441ee16

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 parentfce3b26 commit441ee16

File tree

2 files changed

+65
-30
lines changed

2 files changed

+65
-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
/*
@@ -4025,6 +4035,17 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
40254035
estate->paramLI->parserSetupArg=NULL;/* filled during use */
40264036
estate->paramLI->numParams=estate->ndatums;
40274037

4038+
/* Create the session-wide cast-expression hash if we didn't already */
4039+
if (cast_expr_hash==NULL)
4040+
{
4041+
ctl.keysize=sizeof(plpgsql_CastHashKey);
4042+
ctl.entrysize=sizeof(plpgsql_CastExprHashEntry);
4043+
cast_expr_hash=hash_create("PLpgSQL cast expressions",
4044+
16,/* start small and extend */
4045+
&ctl,
4046+
HASH_ELEM |HASH_BLOBS);
4047+
}
4048+
40284049
/* set up for use of appropriate simple-expression EState and cast hash */
40294050
if (simple_eval_estate)
40304051
{
@@ -4037,27 +4058,21 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
40374058
16,/* start small and extend */
40384059
&ctl,
40394060
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
4040-
estate->cast_hash_context=CurrentMemoryContext;
40414061
}
40424062
else
40434063
{
40444064
estate->simple_eval_estate=shared_simple_eval_estate;
40454065
/* Create the session-wide cast-info hash table if we didn't already */
40464066
if (shared_cast_hash==NULL)
40474067
{
4048-
shared_cast_context=AllocSetContextCreate(TopMemoryContext,
4049-
"PLpgSQL cast info",
4050-
ALLOCSET_DEFAULT_SIZES);
40514068
ctl.keysize=sizeof(plpgsql_CastHashKey);
40524069
ctl.entrysize=sizeof(plpgsql_CastHashEntry);
4053-
ctl.hcxt=shared_cast_context;
40544070
shared_cast_hash=hash_create("PLpgSQL cast cache",
40554071
16,/* start small and extend */
40564072
&ctl,
4057-
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
4073+
HASH_ELEM |HASH_BLOBS);
40584074
}
40594075
estate->cast_hash=shared_cast_hash;
4060-
estate->cast_hash_context=shared_cast_context;
40614076
}
40624077
/* likewise for the simple-expression resource owner */
40634078
if (simple_eval_resowner)
@@ -7768,6 +7783,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
77687783
{
77697784
plpgsql_CastHashKeycast_key;
77707785
plpgsql_CastHashEntry*cast_entry;
7786+
plpgsql_CastExprHashEntry*expr_entry;
77717787
boolfound;
77727788
LocalTransactionIdcurlxid;
77737789
MemoryContextoldcontext;
@@ -7781,10 +7797,28 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
77817797
&cast_key,
77827798
HASH_ENTER,&found);
77837799
if (!found)/* initialize if new entry */
7784-
cast_entry->cast_cexpr=NULL;
7800+
{
7801+
/* We need a second lookup to see if a cast_expr_hash entry exists */
7802+
expr_entry= (plpgsql_CastExprHashEntry*)hash_search(cast_expr_hash,
7803+
&cast_key,
7804+
HASH_ENTER,
7805+
&found);
7806+
if (!found)/* initialize if new expr entry */
7807+
expr_entry->cast_cexpr=NULL;
77857808

7786-
if (cast_entry->cast_cexpr==NULL||
7787-
!cast_entry->cast_cexpr->is_valid)
7809+
cast_entry->cast_centry=expr_entry;
7810+
cast_entry->cast_exprstate=NULL;
7811+
cast_entry->cast_in_use= false;
7812+
cast_entry->cast_lxid=InvalidLocalTransactionId;
7813+
}
7814+
else
7815+
{
7816+
/* Use always-valid link to avoid a second hash lookup */
7817+
expr_entry=cast_entry->cast_centry;
7818+
}
7819+
7820+
if (expr_entry->cast_cexpr==NULL||
7821+
!expr_entry->cast_cexpr->is_valid)
77887822
{
77897823
/*
77907824
* We've not looked up this coercion before, or we have but the cached
@@ -7797,10 +7831,10 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
77977831
/*
77987832
* Drop old cached expression if there is one.
77997833
*/
7800-
if (cast_entry->cast_cexpr)
7834+
if (expr_entry->cast_cexpr)
78017835
{
7802-
FreeCachedExpression(cast_entry->cast_cexpr);
7803-
cast_entry->cast_cexpr=NULL;
7836+
FreeCachedExpression(expr_entry->cast_cexpr);
7837+
expr_entry->cast_cexpr=NULL;
78047838
}
78057839

78067840
/*
@@ -7881,9 +7915,11 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
78817915
((RelabelType*)cast_expr)->arg== (Expr*)placeholder)
78827916
cast_expr=NULL;
78837917

7884-
/* Now we can fill in the hashtable entry. */
7885-
cast_entry->cast_cexpr=cast_cexpr;
7886-
cast_entry->cast_expr= (Expr*)cast_expr;
7918+
/* Now we can fill in the expression hashtable entry. */
7919+
expr_entry->cast_cexpr=cast_cexpr;
7920+
expr_entry->cast_expr= (Expr*)cast_expr;
7921+
7922+
/* Be sure to reset the exprstate hashtable entry, too. */
78877923
cast_entry->cast_exprstate=NULL;
78887924
cast_entry->cast_in_use= false;
78897925
cast_entry->cast_lxid=InvalidLocalTransactionId;
@@ -7892,7 +7928,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
78927928
}
78937929

78947930
/* Done if we have determined that this is a no-op cast. */
7895-
if (cast_entry->cast_expr==NULL)
7931+
if (expr_entry->cast_expr==NULL)
78967932
returnNULL;
78977933

78987934
/*
@@ -7911,7 +7947,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
79117947
if (cast_entry->cast_lxid!=curlxid||cast_entry->cast_in_use)
79127948
{
79137949
oldcontext=MemoryContextSwitchTo(estate->simple_eval_estate->es_query_cxt);
7914-
cast_entry->cast_exprstate=ExecInitExpr(cast_entry->cast_expr,NULL);
7950+
cast_entry->cast_exprstate=ExecInitExpr(expr_entry->cast_expr,NULL);
79157951
cast_entry->cast_in_use= false;
79167952
cast_entry->cast_lxid=curlxid;
79177953
MemoryContextSwitchTo(oldcontext);

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,6 @@ typedef struct PLpgSQL_execstate
10761076

10771077
/* lookup table to use for executing type casts */
10781078
HTAB*cast_hash;
1079-
MemoryContextcast_hash_context;
10801079

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

0 commit comments

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