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.

diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 4630c44e7407c98a2de2df7bafc2753e78bfcfc6..c4960d597e0503a5381023b90529d29de486b24f 100644 (file)
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -68,6 +68,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))
+
 /*
  * This is the head of the backend's list of "saved" CachedPlanSources (i.e.,
  * those that are in long-lived storage and are examined for sinval events).
@@ -352,23 +360,27 @@ CompleteCachedPlan(CachedPlanSource *plansource,
        plansource->query_context = querytree_context;
        plansource->query_list = querytree_list;
 
-       /*
-        * Use the planner machinery to extract dependencies.  Data is saved in
-        * query_context.  (We assume that not a lot of extra cruft is created by
-        * this call.)  We can skip this for one-shot plans.
-        */
-       if (!plansource->is_oneshot)
+       if (!plansource->is_oneshot && !IsTransactionStmtPlan(plansource))
+       {
+               /*
+                * Use the planner machinery to extract dependencies.  Data is saved
+                * in query_context.  (We assume that not a lot of extra cruft is
+                * created by this call.)  We can skip this for one-shot plans, and
+                * transaction control commands have no such dependencies anyway.
+                */
                extract_query_dependencies((Node *) querytree_list,
                                                                   &plansource->relationOids,
                                                                   &plansource->invalItems);
 
-       /*
-        * Also save the current search_path in the query_context.  (This should
-        * not generate much extra cruft either, since almost certainly the path
-        * is already valid.)  Again, don't really need it for one-shot plans.
-        */
-       if (!plansource->is_oneshot)
+               /*
+                * Also save the current search_path in the query_context.  (This
+                * should not generate much extra cruft either, since almost certainly
+                * the path is already valid.)  Again, we don't really need this for
+                * one-shot plans; and we *must* skip this for transaction control
+                * commands, because this could result in catalog accesses.
+                */
                plansource->search_path = GetOverrideSearchPath(querytree_context);
+       }
 
        /*
         * Save the final parameter types (or other parameter specification data)
@@ -542,9 +554,11 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
        /*
         * For one-shot plans, we do not support revalidation checking; it's
         * assumed the query is parsed, planned, and executed in one transaction,
-        * so that no lock re-acquisition is necessary.
+        * so that no lock re-acquisition is necessary.  Also, there is never
+        * any need to revalidate plans for transaction control commands (and
+        * we mustn't risk any catalog accesses when handling those).
         */
-       if (plansource->is_oneshot)
+       if (plansource->is_oneshot || IsTransactionStmtPlan(plansource))
        {
                Assert(plansource->is_valid);
                return NIL;
@@ -967,6 +981,9 @@ choose_custom_plan(CachedPlanSource *plansource, ParamListInfo boundParams)
        /* Otherwise, never any point in a custom plan if there's no parameters */
        if (boundParams == NULL)
                return false;
+       /* ... nor for transaction control statements */
+       if (IsTransactionStmtPlan(plansource))
+               return false;
 
        /* See if caller wants to force the decision */
        if (plansource->cursor_options & CURSOR_OPT_GENERIC_PLAN)
@@ -1622,6 +1639,10 @@ PlanCacheRelCallback(Datum arg, Oid relid)
                if (!plansource->is_valid)
                        continue;
 
+               /* Never invalidate transaction control commands */
+               if (IsTransactionStmtPlan(plansource))
+                       continue;
+
                /*
                 * Check the dependency list for the rewritten querytree.
                 */
@@ -1686,6 +1707,10 @@ PlanCacheFuncCallback(Datum arg, int cacheid, uint32 hashvalue)
                if (!plansource->is_valid)
                        continue;
 
+               /* Never invalidate transaction control commands */
+               if (IsTransactionStmtPlan(plansource))
+                       continue;
+
                /*
                 * Check the dependency list for the rewritten querytree.
                 */
@@ -1775,6 +1800,11 @@ ResetPlanCache(void)
                 * We *must not* mark transaction control statements as invalid,
                 * particularly not ROLLBACK, because they may need to be executed in
                 * aborted transactions when we can't revalidate them (cf bug #5269).
+                */
+               if (IsTransactionStmtPlan(plansource))
+                       continue;
+
+               /*
                 * In general there is no point in invalidating utility statements
                 * since they have no plans anyway.  So invalidate it only if it
                 * contains at least one non-utility statement, or contains a utility