diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2021-02-02 13:49:08 -0500 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2021-02-02 13:49:08 -0500 |
| commit | 2671125c75c04f7abc4a87998959c197e99e34c6 (patch) | |
| tree | f6c5ef494d3bceca9429949860b56a5c6334b22e | |
| parent | 179775135b41a0640ec8b378b2699dca00d3a49a (diff) | |
| download | postgresql-2671125c75c04f7abc4a87998959c197e99e34c6.tar.gz postgresql-2671125c75c04f7abc4a87998959c197e99e34c6.zip | |
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
| -rw-r--r-- | contrib/auto_explain/auto_explain.c | 11 |
1 files changed, 9 insertions, 2 deletions
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c index 4167f7f4d54..13d5aa72757 100644 --- a/contrib/auto_explain/auto_explain.c +++ b/contrib/auto_explain/auto_explain.c @@ -321,9 +321,16 @@ 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.) */ @@ -370,9 +377,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) |