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

Commit2b78d10

Browse files
committed
Fix SQL function execution to be safe with long-lived FmgrInfos.
fmgr_sql had been designed on the assumption that the FmgrInfo it's calledwith has only query lifespan. This is demonstrably unsafe in connectionwith range types, as shown in bug #7881 from Andrew Gierth. Fix thingsso that we re-generate the function's cache data if the (sub)transactionit was made in is no longer active.Back-patch to 9.2. This might be needed further back, but it's not clearwhether the case can realistically arise without range types, so for nowI'll desist from back-patching further.
1 parent891869c commit2b78d10

File tree

3 files changed

+98
-12
lines changed

3 files changed

+98
-12
lines changed

‎src/backend/access/transam/xact.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,27 @@ GetCurrentSubTransactionId(void)
570570
returns->subTransactionId;
571571
}
572572

573+
/*
574+
*SubTransactionIsActive
575+
*
576+
* Test if the specified subxact ID is still active. Note caller is
577+
* responsible for checking whether this ID is relevant to the current xact.
578+
*/
579+
bool
580+
SubTransactionIsActive(SubTransactionIdsubxid)
581+
{
582+
TransactionStates;
583+
584+
for (s=CurrentTransactionState;s!=NULL;s=s->parent)
585+
{
586+
if (s->state==TRANS_ABORT)
587+
continue;
588+
if (s->subTransactionId==subxid)
589+
return true;
590+
}
591+
return false;
592+
}
593+
573594

574595
/*
575596
*GetCurrentCommandId

‎src/backend/executor/functions.c

Lines changed: 76 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@
2525
#include"nodes/nodeFuncs.h"
2626
#include"parser/parse_coerce.h"
2727
#include"parser/parse_func.h"
28+
#include"storage/proc.h"
2829
#include"tcop/utility.h"
2930
#include"utils/builtins.h"
3031
#include"utils/datum.h"
3132
#include"utils/lsyscache.h"
33+
#include"utils/memutils.h"
3234
#include"utils/snapmgr.h"
3335
#include"utils/syscache.h"
3436

@@ -74,8 +76,17 @@ typedef struct execution_state
7476
* and linked to from the fn_extra field of the FmgrInfo struct.
7577
*
7678
* Note that currently this has only the lifespan of the calling query.
77-
* Someday we might want to consider caching the parse/plan results longer
78-
* than that.
79+
* Someday we should rewrite this code to use plancache.c to save parse/plan
80+
* results for longer than that.
81+
*
82+
* Physically, though, the data has the lifespan of the FmgrInfo that's used
83+
* to call the function, and there are cases (particularly with indexes)
84+
* where the FmgrInfo might survive across transactions. We cannot assume
85+
* that the parse/plan trees are good for longer than the (sub)transaction in
86+
* which parsing was done, so we must mark the record with the LXID/subxid of
87+
* its creation time, and regenerate everything if that's obsolete. To avoid
88+
* memory leakage when we do have to regenerate things, all the data is kept
89+
* in a sub-context of the FmgrInfo's fn_mcxt.
7990
*/
8091
typedefstruct
8192
{
@@ -106,6 +117,12 @@ typedef struct
106117
* track of where the original query boundaries are.
107118
*/
108119
List*func_state;
120+
121+
MemoryContextfcontext;/* memory context holding this struct and all
122+
* subsidiary data */
123+
124+
LocalTransactionIdlxid;/* lxid in which cache was made */
125+
SubTransactionIdsubxid;/* subxid in which cache was made */
109126
}SQLFunctionCache;
110127

111128
typedefSQLFunctionCache*SQLFunctionCachePtr;
@@ -551,6 +568,8 @@ static void
551568
init_sql_fcache(FmgrInfo*finfo,Oidcollation,boollazyEvalOK)
552569
{
553570
Oidfoid=finfo->fn_oid;
571+
MemoryContextfcontext;
572+
MemoryContextoldcontext;
554573
Oidrettype;
555574
HeapTupleprocedureTuple;
556575
Form_pg_procprocedureStruct;
@@ -562,7 +581,25 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
562581
Datumtmp;
563582
boolisNull;
564583

584+
/*
585+
* Create memory context that holds all the SQLFunctionCache data.It
586+
* must be a child of whatever context holds the FmgrInfo.
587+
*/
588+
fcontext=AllocSetContextCreate(finfo->fn_mcxt,
589+
"SQL function data",
590+
ALLOCSET_DEFAULT_MINSIZE,
591+
ALLOCSET_DEFAULT_INITSIZE,
592+
ALLOCSET_DEFAULT_MAXSIZE);
593+
594+
oldcontext=MemoryContextSwitchTo(fcontext);
595+
596+
/*
597+
* Create the struct proper, link it to fcontext and fn_extra.Once this
598+
* is done, we'll be able to recover the memory after failure, even if the
599+
* FmgrInfo is long-lived.
600+
*/
565601
fcache= (SQLFunctionCachePtr)palloc0(sizeof(SQLFunctionCache));
602+
fcache->fcontext=fcontext;
566603
finfo->fn_extra= (void*)fcache;
567604

568605
/*
@@ -635,6 +672,11 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
635672
* sublist, for example if the last query rewrites to DO INSTEAD NOTHING.
636673
* (It might not be unreasonable to throw an error in such a case, but
637674
* this is the historical behavior and it doesn't seem worth changing.)
675+
*
676+
* Note: since parsing and planning is done in fcontext, we will generate
677+
* a lot of cruft that lives as long as the fcache does. This is annoying
678+
* but we'll not worry about it until the module is rewritten to use
679+
* plancache.c.
638680
*/
639681
raw_parsetree_list=pg_parse_query(fcache->src);
640682

@@ -700,7 +742,13 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
700742
fcache,
701743
lazyEvalOK);
702744

745+
/* Mark fcache with time of creation to show it's valid */
746+
fcache->lxid=MyProc->lxid;
747+
fcache->subxid=GetCurrentSubTransactionId();
748+
703749
ReleaseSysCache(procedureTuple);
750+
751+
MemoryContextSwitchTo(oldcontext);
704752
}
705753

706754
/* Start up execution of one execution_state node */
@@ -923,9 +971,9 @@ postquel_get_single_result(TupleTableSlot *slot,
923971
Datum
924972
fmgr_sql(PG_FUNCTION_ARGS)
925973
{
926-
MemoryContextoldcontext;
927974
SQLFunctionCachePtrfcache;
928975
ErrorContextCallbacksqlerrcontext;
976+
MemoryContextoldcontext;
929977
boolrandomAccess;
930978
boollazyEvalOK;
931979
boolis_first;
@@ -936,13 +984,6 @@ fmgr_sql(PG_FUNCTION_ARGS)
936984
List*eslist;
937985
ListCell*eslc;
938986

939-
/*
940-
* Switch to context in which the fcache lives. This ensures that
941-
* parsetrees, plans, etc, will have sufficient lifetime. The
942-
* sub-executor is responsible for deleting per-tuple information.
943-
*/
944-
oldcontext=MemoryContextSwitchTo(fcinfo->flinfo->fn_mcxt);
945-
946987
/*
947988
* Setup error traceback support for ereport()
948989
*/
@@ -978,20 +1019,43 @@ fmgr_sql(PG_FUNCTION_ARGS)
9781019
}
9791020

9801021
/*
981-
* Initialize fcache (build plans) if first time through.
1022+
* Initialize fcache (build plans) if first time through; or re-initialize
1023+
* if the cache is stale.
9821024
*/
9831025
fcache= (SQLFunctionCachePtr)fcinfo->flinfo->fn_extra;
1026+
1027+
if (fcache!=NULL)
1028+
{
1029+
if (fcache->lxid!=MyProc->lxid||
1030+
!SubTransactionIsActive(fcache->subxid))
1031+
{
1032+
/* It's stale; unlink and delete */
1033+
fcinfo->flinfo->fn_extra=NULL;
1034+
MemoryContextDelete(fcache->fcontext);
1035+
fcache=NULL;
1036+
}
1037+
}
1038+
9841039
if (fcache==NULL)
9851040
{
9861041
init_sql_fcache(fcinfo->flinfo,PG_GET_COLLATION(),lazyEvalOK);
9871042
fcache= (SQLFunctionCachePtr)fcinfo->flinfo->fn_extra;
9881043
}
989-
eslist=fcache->func_state;
1044+
1045+
/*
1046+
* Switch to context in which the fcache lives. This ensures that our
1047+
* tuplestore etc will have sufficient lifetime. The sub-executor is
1048+
* responsible for deleting per-tuple information.(XXX in the case of a
1049+
* long-lived FmgrInfo, this policy represents more memory leakage, but
1050+
* it's not entirely clear where to keep stuff instead.)
1051+
*/
1052+
oldcontext=MemoryContextSwitchTo(fcache->fcontext);
9901053

9911054
/*
9921055
* Find first unfinished query in function, and note whether it's the
9931056
* first query.
9941057
*/
1058+
eslist=fcache->func_state;
9951059
es=NULL;
9961060
is_first= true;
9971061
foreach(eslc,eslist)

‎src/include/access/xact.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ extern TransactionId GetCurrentTransactionId(void);
215215
externTransactionIdGetCurrentTransactionIdIfAny(void);
216216
externTransactionIdGetStableLatestTransactionId(void);
217217
externSubTransactionIdGetCurrentSubTransactionId(void);
218+
externboolSubTransactionIsActive(SubTransactionIdsubxid);
218219
externCommandIdGetCurrentCommandId(boolused);
219220
externTimestampTzGetCurrentTransactionStartTimestamp(void);
220221
externTimestampTzGetCurrentStatementStartTimestamp(void);

0 commit comments

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