postgres.git / summary / log / commit / refs
commit c13ed70a4f26bae435c297ae09b1be9f50f2eede
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat Apr 20 20:59:31 2013 +0000
First released in: 9.3.0, 8.4.18, 9.0.14, 9.1.10, 9.2.5
Fix longstanding race condition in plancache.c.
When creating or manipulating a cached plan for a transaction control
command (particularly ROLLBACK), we must not perform any catalog accesses,
since we might be in an aborted transaction. However, plancache.c busily
saved or examined the search_path for every cached plan. If we were
unlucky enough to do this at a moment where the path's expansion into
schema OIDs wasn't already cached, we'd do some catalog accesses; and with
some more bad luck such as an ill-timed signal arrival, that could lead to
crashes or Assert failures, as exhibited in bug #8095 from Nachiket Vaidya.
Fortunately, there's no real need to consider the search path for such
commands, so we can just skip the relevant steps when the subject statement
is a TransactionStmt. This is somewhat related to bug #5269, though the
failure happens during initial cached-plan creation rather than
revalidation.
This bug has been there since the plan cache was invented, so back-patch
to all supported branches.
src/backend/utils/cache/plancache.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 08ddfa9bcba..5dcea3db33a 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -61,6 +61,14 @@
#include "utils/syscache.h"
+/*
+ * We must skip "overhead" operations that involve database access when the
+ * cached plan's subject statement is a transaction control command.
+ */
+#define IsTransactionStmtPlan(plansource) \
+ ((plansource)->raw_parse_tree && \
+ IsA((plansource)->raw_parse_tree, TransactionStmt))
+
static List *cached_plans_list = NIL;
static void StoreCachedPlan(CachedPlanSource *plansource, List *stmt_list,
@@ -136,9 +144,13 @@ CreateCachedPlan(Node *raw_parse_tree,
/*
* Fetch current search_path into new context, but do any recalculation
- * work required in caller's context.
+ * work required in caller's context. Skip this for a transaction control
+ * command, since we won't need it and can't risk catalog access.
*/
- search_path = GetOverrideSearchPath(source_context);
+ if (raw_parse_tree && IsA(raw_parse_tree, TransactionStmt))
+ search_path = NULL;
+ else
+ search_path = GetOverrideSearchPath(source_context);
/*
* Create and fill the CachedPlanSource struct within the new context.
@@ -229,9 +241,13 @@ FastCreateCachedPlan(Node *raw_parse_tree,
/*
* Fetch current search_path into given context, but do any recalculation
- * work required in caller's context.
+ * work required in caller's context. Skip this for a transaction control
+ * command, since we won't need it and can't risk catalog access.
*/
- search_path = GetOverrideSearchPath(context);
+ if (raw_parse_tree && IsA(raw_parse_tree, TransactionStmt))
+ search_path = NULL;
+ else
+ search_path = GetOverrideSearchPath(context);
/*
* Create and fill the CachedPlanSource struct within the given context.
@@ -517,7 +533,8 @@ RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner)
*
* (XXX is there anything else we really need to restore?)
*/
- PushOverrideSearchPath(plansource->search_path);
+ if (plansource->search_path)
+ PushOverrideSearchPath(plansource->search_path);
/*
* If a snapshot is already set (the normal case), we can just use
@@ -601,7 +618,8 @@ RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner)
PopActiveSnapshot();
/* Now we can restore current search path */
- PopOverrideSearchPath();
+ if (plansource->search_path)
+ PopOverrideSearchPath();
/*
* Store the plans into the plancache entry, advancing the generation
[parent: d189deebd3c9]