Skip to content

P
Postgres FD Implementation
Commit 9bd0feeb authored by Stephen Frost's avatar Stephen Frost
Browse files
Options

Improvements to GetErrorContextStack() 

As GetErrorContextStack() borrowed setup and tear-down code from other
places, it was less than clear that it must only be called as a
top-level entry point into the error system and can't be called by an
exception handler (unlike the rest of the error system, which is set up
to be reentrant-safe).

Being called from an exception handler is outside the charter of
GetErrorContextStack(), so add a bit more protection against it,
improve the comments addressing why we have to set up an errordata
stack for this function at all, and add a few more regression tests.

Lack of clarity pointed out by Tom Lane; all bugs are mine.
parent e4c6cccd
Hide whitespace changes
Inline Side-by-side
Showing with 105 additions and 25 deletions
src/backend/utils/error/elog.c
View file @ 9bd0feeb
......@@ -1627,12 +1627,16 @@ pg_re_throw(void)
/*
* GetErrorContextStack - Return the error context stack
* GetErrorContextStack - Return the context stack, for display/diags
*
* Returns a pstrdup'd string in the caller's context which includes the full
* Returns a pstrdup'd string in the caller's context which includes the PG
* call stack. It is the caller's responsibility to ensure this string is
* pfree'd (or its context cleaned up) when done.
*
* Note that this function may *not* be called from any existing error case
* and is not for error-reporting (use ereport() and friends instead, which
* will also produce a stack trace).
*
* This information is collected by traversing the error contexts and calling
* each context's callback function, each of which is expected to call
* errcontext() to return a string which can be presented to the user.
......@@ -1648,22 +1652,36 @@ GetErrorContextStack(void)
/* this function should not be called from an exception handler */
Assert(recursion_depth == 0);
/* Check that we have enough room on the stack for ourselves */
if (++errordata_stack_depth >= ERRORDATA_STACK_SIZE)
/*
* This function should never be called from an exception handler and
* therefore will only ever be the top item on the errordata stack
* (which is set up so that the calls to the callback functions are
* able to use it).
*
* Better safe than sorry, so double-check that we are not being called
* from an exception handler.
*/
if (errordata_stack_depth != -1)
{
/*
* Stack not big enough.. Something bad has happened, therefore
* PANIC as we may be in an infinite loop.
*/
errordata_stack_depth = -1; /* make room on stack */
ereport(PANIC, (errmsg_internal("ERRORDATA_STACK_SIZE exceeded")));
ereport(PANIC,
(errmsg_internal("GetErrorContextStack called from exception handler")));
}
/* Initialize data for this error frame */
/*
* Initialize data for the top, and only at this point, error frame as the
* callback functions we're about to call will turn around and call
* errcontext(), which expects to find a valid errordata stack.
*/
errordata_stack_depth = 0;
edata = &errordata[errordata_stack_depth];
MemSet(edata, 0, sizeof(ErrorData));
/* Use ErrorContext as a short lived context for the callbacks */
/*
* Use ErrorContext as a short lived context for calling the callbacks;
* the callbacks will use it through errcontext() even if we don't call
* them with it, so we have to clean it up below either way.
*/
MemoryContextSwitchTo(ErrorContext);
/*
......@@ -1671,7 +1689,8 @@ GetErrorContextStack(void)
* into edata->context.
*
* Errors occurring in callback functions should go through the regular
* error handling code which should handle any recursive errors.
* error handling code which should handle any recursive errors and must
* never call back to us.
*/
for (econtext = error_context_stack;
econtext != NULL;
......@@ -1688,7 +1707,12 @@ GetErrorContextStack(void)
if (edata->context)
result = pstrdup(edata->context);
/* Reset error stack */
/*
* Reset error stack- note that we should be the only item on the error
* stack at this point and therefore it's safe to clean up the whole stack.
* This function is not intended nor able to be called from exception
* handlers.
*/
FlushErrorState();
return result;
......
src/test/regress/expected/plpgsql.out
View file @ 9bd0feeb
......@@ -4904,27 +4904,55 @@ declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
-- lets do it again, just for fun..
get diagnostics _context = pg_context;
raise notice '***%***', _context;
raise notice 'lets make sure we didnt break anything';
return 2 * $1;
end;
$$ language plpgsql;
create or replace function outer_func(int)
returns int as $$
declare
myresult int;
begin
return inner_func($1);
raise notice 'calling down into inner_func()';
myresult := inner_func($1);
raise notice 'inner_func() done';
return myresult;
end;
$$ language plpgsql;
create or replace function outer_outer_func(int)
returns int as $$
declare
myresult int;
begin
return outer_func($1);
raise notice 'calling down into outer_func()';
myresult := outer_func($1);
raise notice 'outer_func() done';
return myresult;
end;
$$ language plpgsql;
select outer_outer_func(10);
NOTICE: calling down into outer_func()
NOTICE: calling down into inner_func()
CONTEXT: PL/pgSQL function outer_outer_func(integer) line 6 at assignment
NOTICE: ***PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_func(integer) line 6 at assignment
PL/pgSQL function outer_outer_func(integer) line 6 at assignment***
CONTEXT: PL/pgSQL function outer_func(integer) line 6 at assignment
PL/pgSQL function outer_outer_func(integer) line 6 at assignment
NOTICE: ***PL/pgSQL function inner_func(integer) line 7 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 6 at assignment
PL/pgSQL function outer_outer_func(integer) line 6 at assignment***
CONTEXT: PL/pgSQL function outer_func(integer) line 6 at assignment
PL/pgSQL function outer_outer_func(integer) line 6 at assignment
NOTICE: lets make sure we didnt break anything
CONTEXT: PL/pgSQL function outer_func(integer) line 6 at assignment
PL/pgSQL function outer_outer_func(integer) line 6 at assignment
NOTICE: inner_func() done
CONTEXT: PL/pgSQL function outer_outer_func(integer) line 6 at assignment
NOTICE: outer_func() done
outer_outer_func
------------------
20
......@@ -4932,11 +4960,25 @@ PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
-- repeated call should to work
select outer_outer_func(20);
NOTICE: calling down into outer_func()
NOTICE: calling down into inner_func()
CONTEXT: PL/pgSQL function outer_outer_func(integer) line 6 at assignment
NOTICE: ***PL/pgSQL function inner_func(integer) line 4 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN***
CONTEXT: PL/pgSQL function outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_outer_func(integer) line 3 at RETURN
PL/pgSQL function outer_func(integer) line 6 at assignment
PL/pgSQL function outer_outer_func(integer) line 6 at assignment***
CONTEXT: PL/pgSQL function outer_func(integer) line 6 at assignment
PL/pgSQL function outer_outer_func(integer) line 6 at assignment
NOTICE: ***PL/pgSQL function inner_func(integer) line 7 at GET DIAGNOSTICS
PL/pgSQL function outer_func(integer) line 6 at assignment
PL/pgSQL function outer_outer_func(integer) line 6 at assignment***
CONTEXT: PL/pgSQL function outer_func(integer) line 6 at assignment
PL/pgSQL function outer_outer_func(integer) line 6 at assignment
NOTICE: lets make sure we didnt break anything
CONTEXT: PL/pgSQL function outer_func(integer) line 6 at assignment
PL/pgSQL function outer_outer_func(integer) line 6 at assignment
NOTICE: inner_func() done
CONTEXT: PL/pgSQL function outer_outer_func(integer) line 6 at assignment
NOTICE: outer_func() done
outer_outer_func
------------------
40
......
src/test/regress/sql/plpgsql.sql
View file @ 9bd0feeb
......@@ -3888,21 +3888,35 @@ declare _context text;
begin
get diagnostics _context = pg_context;
raise notice '***%***', _context;
-- lets do it again, just for fun..
get diagnostics _context = pg_context;
raise notice '***%***', _context;
raise notice 'lets make sure we didnt break anything';
return 2 * $1;
end;
$$ language plpgsql;
create or replace function outer_func(int)
returns int as $$
declare
myresult int;
begin
return inner_func($1);
raise notice 'calling down into inner_func()';
myresult := inner_func($1);
raise notice 'inner_func() done';
return myresult;
end;
$$ language plpgsql;
create or replace function outer_outer_func(int)
returns int as $$
declare
myresult int;
begin
return outer_func($1);
raise notice 'calling down into outer_func()';
myresult := outer_func($1);
raise notice 'outer_func() done';
return myresult;
end;
$$ language plpgsql;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment