]> granicus.if.org Git - postgresql/commitdiff
Fix longstanding race condition in plancache.c.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 20 Apr 2013 20:59:36 +0000 (16:59 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 20 Apr 2013 20:59:36 +0000 (16:59 -0400)
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

index 06e254912bf8fb6f0f173ced0cf4425def17f79e..76ee3e1f9ecef014d8292acb5fb91e5cea447941 100644 (file)
 #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