Fix ancient memory leak in contrib/auto_explain.

tglsfdc · tglsfdc · commit f0f9ed68d3f5 · 2021-02-02T13:49:08.000-05:00
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
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
@@ -282,8 +282,15 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
 {
 	if (queryDesc->totaltime && auto_explain_enabled())
 	{
+		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.)
@@ -331,9 +338,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)
