Re: alternative to PG_CATCH

[Peter Eisentraut]

On 2019-11-04 16:01, Tom Lane wrote:

Now that I've actually looked at the patched code, there's a far
more severe problem with it.  Namely, that use of PG_FINALLY
means that the "finally" segment is run without having popped
the error context stack, which means that any error thrown
within that segment will sigsetjmp right back to the top,
resulting in an infinite loop.  (Well, not infinite, because
it'll crash out once the error nesting depth limit is hit.)
We *must* pop the stack before running recovery code.

I can confirm that that indeed happens. :(

Here is a patch to fix it.

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

From aa758de0919e2356bf46390b89d8b17d20b8c09e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 5 Nov 2019 22:01:18 +0100
Subject: [PATCH] Fix nested error handling in PG_FINALLY

We need to pop the error stack before running the user-supplied
PG_FINALLY code.  Otherwise an error in the cleanup code would end up
at the same sigsetjmp() invocation and result in an infinite error
handling loop.
---
 src/backend/utils/adt/xml.c |  2 +-
 src/include/utils/elog.h    | 11 ++++++++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 3a493dd6bf..2256c24137 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -3864,7 +3864,7 @@ xml_xmlnodetoxmltype(xmlNodePtr cur, PgXmlErrorContext 
*xmlerrcxt)
 
                        result = xmlBuffer_to_xmltype(buf);
                }
-               PG_FINALLY()
+               PG_FINALLY();
                {
                        if (nodefree)
                                nodefree(cur_copy);
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 853c2e0709..93f26876b0 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -338,14 +338,19 @@ extern PGDLLIMPORT ErrorContextCallback 
*error_context_stack;
                } \
                else \
                        _do_rethrow = true; \
-               {
+               { \
+                       PG_exception_stack = _save_exception_stack; \
+                       error_context_stack = _save_context_stack
 
 #define PG_END_TRY()  \
                } \
-               PG_exception_stack = _save_exception_stack; \
-               error_context_stack = _save_context_stack; \
                if (_do_rethrow) \
                                PG_RE_THROW(); \
+               else \
+               { \
+                       PG_exception_stack = _save_exception_stack; \
+                       error_context_stack = _save_context_stack; \
+               } \
        } while (0)
 
 /*
-- 
2.23.0