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

Commitd4f4bdf

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 parentbf63c4a commitd4f4bdf

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
@@ -24,10 +24,12 @@
2424
#include"nodes/nodeFuncs.h"
2525
#include"parser/parse_coerce.h"
2626
#include"parser/parse_func.h"
27+
#include"storage/proc.h"
2728
#include"tcop/utility.h"
2829
#include"utils/builtins.h"
2930
#include"utils/datum.h"
3031
#include"utils/lsyscache.h"
32+
#include"utils/memutils.h"
3133
#include"utils/snapmgr.h"
3234
#include"utils/syscache.h"
3335

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

110127
typedefSQLFunctionCache*SQLFunctionCachePtr;
@@ -550,6 +567,8 @@ static void
550567
init_sql_fcache(FmgrInfo*finfo,Oidcollation,boollazyEvalOK)
551568
{
552569
Oidfoid=finfo->fn_oid;
570+
MemoryContextfcontext;
571+
MemoryContextoldcontext;
553572
Oidrettype;
554573
HeapTupleprocedureTuple;
555574
Form_pg_procprocedureStruct;
@@ -561,7 +580,25 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
561580
Datumtmp;
562581
boolisNull;
563582

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

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

@@ -699,7 +741,13 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
699741
fcache,
700742
lazyEvalOK);
701743

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

705753
/* Start up execution of one execution_state node */
@@ -922,9 +970,9 @@ postquel_get_single_result(TupleTableSlot *slot,
922970
Datum
923971
fmgr_sql(PG_FUNCTION_ARGS)
924972
{
925-
MemoryContextoldcontext;
926973
SQLFunctionCachePtrfcache;
927974
ErrorContextCallbacksqlerrcontext;
975+
MemoryContextoldcontext;
928976
boolrandomAccess;
929977
boollazyEvalOK;
930978
boolis_first;
@@ -935,13 +983,6 @@ fmgr_sql(PG_FUNCTION_ARGS)
935983
List*eslist;
936984
ListCell*eslc;
937985

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

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

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

‎src/include/access/xact.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ extern TransactionId GetCurrentTransactionId(void);
212212
externTransactionIdGetCurrentTransactionIdIfAny(void);
213213
externTransactionIdGetStableLatestTransactionId(void);
214214
externSubTransactionIdGetCurrentSubTransactionId(void);
215+
externboolSubTransactionIsActive(SubTransactionIdsubxid);
215216
externCommandIdGetCurrentCommandId(boolused);
216217
externTimestampTzGetCurrentTransactionStartTimestamp(void);
217218
externTimestampTzGetCurrentStatementStartTimestamp(void);

0 commit comments

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