Fix ancient memory leak in contrib/auto_explain.
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
This commit is contained in:
parent
d1ab4bf6ed
commit
5fc5ff61ce
@ -322,8 +322,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.)
|
||||
@ -373,9 +380,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)
|
||||
|
Loading…
Reference in New Issue
Block a user