From 9cf163266017ec6d0e190313cce1d417cbf4a549 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 16 Jun 2021 19:30:17 -0400 Subject: [PATCH] Fix plancache refcount leak after error in ExecuteQuery. When stuffing a plan from the plancache into a Portal, one is not supposed to risk throwing an error between GetCachedPlan and PortalDefineQuery; if that happens, the plan refcount incremented by GetCachedPlan will be leaked. I managed to break this rule while refactoring code in 9dbf2b7d7. There is no visible consequence other than some memory leakage, and since nobody is very likely to trigger the relevant error conditions many times in a row, it's not surprising we haven't noticed. Nonetheless, it's a bug, so rearrange the order of operations to remove the hazard. Noted on the way to looking for a better fix for bug #17053. This mistake is pretty old, so back-patch to all supported branches. --- src/backend/commands/prepare.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c index b945b1556a..63aa47f067 100644 --- a/src/backend/commands/prepare.c +++ b/src/backend/commands/prepare.c @@ -246,6 +246,17 @@ ExecuteQuery(ExecuteStmt *stmt, IntoClause *intoClause, cplan = GetCachedPlan(entry->plansource, paramLI, false, NULL); plan_list = cplan->stmt_list; + /* + * DO NOT add any logic that could possibly throw an error between + * GetCachedPlan and PortalDefineQuery, or you'll leak the plan refcount. + */ + PortalDefineQuery(portal, + NULL, + query_string, + entry->plansource->commandTag, + plan_list, + cplan); + /* * For CREATE TABLE ... AS EXECUTE, we must verify that the prepared * statement is one that produces tuples. Currently we insist that it be @@ -289,13 +300,6 @@ ExecuteQuery(ExecuteStmt *stmt, IntoClause *intoClause, count = FETCH_ALL; } - PortalDefineQuery(portal, - NULL, - query_string, - entry->plansource->commandTag, - plan_list, - cplan); - /* * Run the portal as appropriate. */