From 17de532c3f8d4b1137ce4215ecd246b38ce29914 Mon Sep 17 00:00:00 2001 From: Ted Yu Date: Tue, 2 Feb 2021 15:32:01 -0800 Subject: [PATCH] [#7070] ysql: Import 'Fix ancient memory leak in contrib/auto_explain.' Summary: Upstream commit was 5c0f7cc5442108e113d4fb88c952329b467e2c6a Commit message was: ``` The ExecutorEnd hook is invoked in a context that could be quite long-lived, not the executor's own per-query context as I think we were sort of assuming. Thus, any cruft generated while producing the EXPLAIN output could accumulate over multiple queries. This can result in spectacular leakage if log_nested_statements is on, and even without that I'm surprised nobody complained before. To fix, just switch into the executor's context so that anything we allocate will be released when standard_ExecutorEnd frees the executor state. We might as well nuke the code's retail pfree of the explain output string, too; that's laughably inadequate to the need. Japin Li, per report from Jeff Janes. This bug is old, so back-patch to all supported branches. Discussion: https://postgr.es/m/CAMkU=1wCVtbeRn0s9gt12KwQ7PLXovbpM8eg25SYocKW3BT4hg@mail.gmail.com ``` Test Plan: Build Yugabyte DB and run test suite via Jenkins Reviewers: jason Reviewed By: jason Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D10529 --- src/postgres/contrib/auto_explain/auto_explain.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/postgres/contrib/auto_explain/auto_explain.c b/src/postgres/contrib/auto_explain/auto_explain.c index 03be05129ffe..4a2cc4b01d85 100644 --- a/src/postgres/contrib/auto_explain/auto_explain.c +++ b/src/postgres/contrib/auto_explain/auto_explain.c @@ -309,8 +309,15 @@ explain_ExecutorEnd(QueryDesc *queryDesc) { if (queryDesc->totaltime && auto_explain_enabled() && current_query_sampled) { + MemoryContext oldcxt; double msec; + /* + * Make sure we operate in the per-query context, so any cruft will be + * discarded later during ExecutorEnd. + */ + oldcxt = MemoryContextSwitchTo(queryDesc->estate->es_query_cxt); + /* * Make sure stats accumulation is done. (Note: it's okay if several * levels of hook all do this.) @@ -360,9 +367,9 @@ explain_ExecutorEnd(QueryDesc *queryDesc) (errmsg("duration: %.3f ms plan:\n%s", msec, es->str->data), errhidestmt(true))); - - pfree(es->str->data); } + + MemoryContextSwitchTo(oldcxt); } if (prev_ExecutorEnd)