1
0
mirror of https://github.com/postgres/postgres.git synced 2025-05-03 22:24:49 +03:00

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  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 , 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.
This commit is contained in:
Tom Lane 2013-04-20 16:59:21 -04:00
parent 61b962345d
commit ac63dca607

@ -68,6 +68,14 @@
#include "utils/syscache.h" #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., * 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). * 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_context = querytree_context;
plansource->query_list = querytree_list; plansource->query_list = querytree_list;
/* 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. * Use the planner machinery to extract dependencies. Data is saved
*/ * in query_context. (We assume that not a lot of extra cruft is
if (!plansource->is_oneshot) * 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, extract_query_dependencies((Node *) querytree_list,
&plansource->relationOids, &plansource->relationOids,
&plansource->invalItems); &plansource->invalItems);
/* /*
* Also save the current search_path in the query_context. (This should * Also save the current search_path in the query_context. (This
* not generate much extra cruft either, since almost certainly the path * should not generate much extra cruft either, since almost certainly
* is already valid.) Again, don't really need it for one-shot plans. * the path is already valid.) Again, we don't really need this for
*/ * one-shot plans; and we *must* skip this for transaction control
if (!plansource->is_oneshot) * commands, because this could result in catalog accesses.
*/
plansource->search_path = GetOverrideSearchPath(querytree_context); plansource->search_path = GetOverrideSearchPath(querytree_context);
}
/* /*
* Save the final parameter types (or other parameter specification data) * 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 * For one-shot plans, we do not support revalidation checking; it's
* assumed the query is parsed, planned, and executed in one transaction, * 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); Assert(plansource->is_valid);
return NIL; 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 */ /* Otherwise, never any point in a custom plan if there's no parameters */
if (boundParams == NULL) if (boundParams == NULL)
return false; return false;
/* ... nor for transaction control statements */
if (IsTransactionStmtPlan(plansource))
return false;
/* See if caller wants to force the decision */ /* See if caller wants to force the decision */
if (plansource->cursor_options & CURSOR_OPT_GENERIC_PLAN) if (plansource->cursor_options & CURSOR_OPT_GENERIC_PLAN)
@ -1622,6 +1639,10 @@ PlanCacheRelCallback(Datum arg, Oid relid)
if (!plansource->is_valid) if (!plansource->is_valid)
continue; continue;
/* Never invalidate transaction control commands */
if (IsTransactionStmtPlan(plansource))
continue;
/* /*
* Check the dependency list for the rewritten querytree. * Check the dependency list for the rewritten querytree.
*/ */
@ -1686,6 +1707,10 @@ PlanCacheFuncCallback(Datum arg, int cacheid, uint32 hashvalue)
if (!plansource->is_valid) if (!plansource->is_valid)
continue; continue;
/* Never invalidate transaction control commands */
if (IsTransactionStmtPlan(plansource))
continue;
/* /*
* Check the dependency list for the rewritten querytree. * Check the dependency list for the rewritten querytree.
*/ */
@ -1775,6 +1800,11 @@ ResetPlanCache(void)
* We *must not* mark transaction control statements as invalid, * We *must not* mark transaction control statements as invalid,
* particularly not ROLLBACK, because they may need to be executed in * particularly not ROLLBACK, because they may need to be executed in
* aborted transactions when we can't revalidate them (cf bug #5269). * 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 * In general there is no point in invalidating utility statements
* since they have no plans anyway. So invalidate it only if it * since they have no plans anyway. So invalidate it only if it
* contains at least one non-utility statement, or contains a utility * contains at least one non-utility statement, or contains a utility