From dd9c6ff468fbc7cce97f100bdbca6be6e13148f5 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 20 Apr 2013 16:59:36 -0400 Subject: [PATCH] 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 06e254912b..76ee3e1f9e 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 -- 2.40.0