]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix ancient memory leak in contrib/auto_explain.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 2 Feb 2021 18:49:08 +0000 (13:49 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 2 Feb 2021 18:49:08 +0000 (13:49 -0500)
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

contrib/auto_explain/auto_explain.c

index 6b9795c6ac08d53996be713d925630c22e1cdc82..4450c8f2a26b4c860222a38135f9040c69a7c6dc 100644 (file)
@@ -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)