Обсуждение: No Callbacks on FATAL

Hi,

On 2023-01-11 17:47:28 -0500, Ed Behn wrote:
> I'm developing a module that implements Haskell as a procedural language (
> https://www.postgresql.org/about/news/plhaskell-v10-released-2519/)
> 
> I'm using a callback function that is called when a memory context is
> deleted to remove a temporary file. This works fine when the transaction
> ends normally or raises an ERROR. However, when a FATAL event happens, the
> callback is not run. Is this a bug or intended behaviour? I think that this
> is a new behavior and that the callback was called in an earlier version
> (perhaps v14) when I was originally developing this code. I'm running
> v15.1.
> 
> It seems to me that callbacks should be run in the event of a FATAL event
> in order to clean up any lingering issues.

I think you need to provide a bit more details to allow us to analyze this. I
assume you're talking about a MemoryContextRegisterResetCallback()?  Which
memory context are you registering the callback on? What FATAL error is
preventing the cleanup from happening?

Even better would be a way to reproduce this without needing to build an
external extension with its own dependencies. Perhaps you can hack it into one
of the contrib/ modules?

Greetings,

Andres Freund

Ed Behn <ed@behn.us> writes:
> I'm using a callback function that is called when a memory context is
> deleted to remove a temporary file. This works fine when the transaction
> ends normally or raises an ERROR. However, when a FATAL event happens, the
> callback is not run. Is this a bug or intended behaviour?

It's intended behavior, and I seriously doubt that it ever worked
differently.

> It seems to me that callbacks should be run in the event of a FATAL event
> in order to clean up any lingering issues.

They'd be far more likely to cause issues than cure them.  Or at least
that's the design assumption.  If you really need something here, put
it in an on_proc_exit callback not a memory context callback.

            regards, tom lane

Hi,

On 2023-01-11 18:10:33 -0500, Tom Lane wrote:
> Ed Behn <ed@behn.us> writes:
> > I'm using a callback function that is called when a memory context is
> > deleted to remove a temporary file. This works fine when the transaction
> > ends normally or raises an ERROR. However, when a FATAL event happens, the
> > callback is not run. Is this a bug or intended behaviour?
>
> It's intended behavior, and I seriously doubt that it ever worked
> differently.

Hm? MemoryContextDelete() unconditionally calls the
callbacks. ShutdownPostgres() calls AbortOutOfAnyTransaction(). So if there's
an ongoing transaction, we'll call the reset callbacks on TopMemoryContext and
its children.

Of course that doesn't mean we'll delete all existing contexts...


> > It seems to me that callbacks should be run in the event of a FATAL event
> > in order to clean up any lingering issues.
>
> They'd be far more likely to cause issues than cure them.  Or at least
> that's the design assumption.  If you really need something here, put
> it in an on_proc_exit callback not a memory context callback.

Or, depending on the use case, a transaction callback.

It's really hard to know what precisely to suggest, without knowing a good bit
more about the intended usecase.


Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2023-01-11 18:10:33 -0500, Tom Lane wrote:
>> It's intended behavior, and I seriously doubt that it ever worked
>> differently.

> Hm? MemoryContextDelete() unconditionally calls the
> callbacks. ShutdownPostgres() calls AbortOutOfAnyTransaction(). So if there's
> an ongoing transaction, we'll call the reset callbacks on TopMemoryContext and
> its children.

Hmm ... I'd forgotten that we'd reach AbortOutOfAnyTransaction in
the FATAL code path.  It does seem like any memory contexts below
TopTransactionContext ought to get cleaned up then.

As you say, we really need more details to see what's happening
here.

            regards, tom lane

Hi hackers,

> > Hm? MemoryContextDelete() unconditionally calls the
> > callbacks. ShutdownPostgres() calls AbortOutOfAnyTransaction(). So if there's
> > an ongoing transaction, we'll call the reset callbacks on TopMemoryContext and
> > its children.
>
> Hmm ... I'd forgotten that we'd reach AbortOutOfAnyTransaction in
> the FATAL code path.  It does seem like any memory contexts below
> TopTransactionContext ought to get cleaned up then.

I wonder if this is a desired behavior. FATAL means a critical error
local to a given backend, but not affecting shared memory, right? Is
it generally safe to execute context memory callbacks having a FATAL
error?

> As you say, we really need more details to see what's happening here.

Yep, minimal steps to reproduce the issue would be much appreciated!

-- 
Best regards,
Aleksander Alekseev

Hi,

On 2023-01-13 16:14:11 +0300, Aleksander Alekseev wrote:
> > > Hm? MemoryContextDelete() unconditionally calls the
> > > callbacks. ShutdownPostgres() calls AbortOutOfAnyTransaction(). So if there's
> > > an ongoing transaction, we'll call the reset callbacks on TopMemoryContext and
> > > its children.
> >
> > Hmm ... I'd forgotten that we'd reach AbortOutOfAnyTransaction in
> > the FATAL code path.  It does seem like any memory contexts below
> > TopTransactionContext ought to get cleaned up then.
> 
> I wonder if this is a desired behavior. FATAL means a critical error
> local to a given backend, but not affecting shared memory, right? Is
> it generally safe to execute context memory callbacks having a FATAL
> error?

We need to roll back the in-progress transaction, otherwise we'd be in
trouble. Some resets are part of that. If the error actually corrupted local
state badly enough to break the transaction machinery, we'd need to PANIC out.

Greetings,

Andres Freund