Clean up memory leakage that occurs in context callback functions.

An error context callback function might leak some memory into
ErrorContext, since those functions are run with ErrorContext as
current context.  In the case where the elevel is ERROR, this is
no problem since the code level that catches the error should do
FlushErrorState to clean up, and that will reset ErrorContext.
However, if the elevel is less than ERROR then no such cleanup occurs.
In principle, repeated leaks while emitting log messages or client
notices could accumulate arbitrarily much leaked data, if no ERROR
occurs in the session.

To fix, let errfinish() perform an ErrorContext reset if it is
at the outermost error nesting level.  (If it isn't, we'll delay
cleanup until the outermost nesting level is exited.)

The only actual leakage of this sort that I've been able to observe
within our regression tests was recently introduced by commit
f727b63e8.  While it seems plausible that there are other such
leaks not reached in the regression tests, the lack of field
reports suggests that they're not a big problem.  Accordingly,
I won't take the risk of back-patching this now.  We can always
back-patch later if we get field reports of leaks.

Reported-by: Andres Freund <andres@anarazel.de>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/jngsjonyfscoont4tnwi2qoikatpd5hifsg373vmmjvugwiu6g@m6opxh7uisgd

1 files changed, 13 insertions, 4 deletions

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index b7b9692f8c8..648d2d2e70c 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -542,11 +542,20 @@ errfinish(const char *filename, int lineno, const char *funcname)
 	/* Emit the message to the right places */
 	EmitErrorReport();
 
-	/* Now free up subsidiary data attached to stack entry, and release it */
-	FreeErrorDataContents(edata);
-	errordata_stack_depth--;
+	/*
+	 * If this is the outermost recursion level, we can clean up by resetting
+	 * ErrorContext altogether (compare FlushErrorState), which is good
+	 * because it cleans up any random leakages that might have occurred in
+	 * places such as context callback functions.  If we're nested, we can
+	 * only safely remove the subsidiary data of the current stack entry.
+	 */
+	if (errordata_stack_depth == 0 && recursion_depth == 1)
+		MemoryContextReset(ErrorContext);
+	else
+		FreeErrorDataContents(edata);
 
-	/* Exit error-handling context */
+	/* Release stack entry and exit error-handling context */
+	errordata_stack_depth--;
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;