On 05/01/2024 19:23, Robert Haas wrote:
On Fri, Nov 24, 2023 at 10:47 AM Heikki Linnakangas <hlinn...@iki.fi> wrote:
What do you think?
At least for 0001 and 0002, I think we should just add the stack depth checks.
With regard to 0001, CommitTransactionCommand() and friends are hard
enough to understand as it is; they need "goto" like I need an extra
hole in my head.
With regard to 0002, this function isn't sufficiently important to
justify adding special-case code for an extremely rare event. We
should just handle it the way we do in general.
I agree that in the memory-context case it might be worth expending
some more code to be more clever. But I probably wouldn't do that for
MemoryContextStats(); check_stack_depth() seems fine for that one.
In general, I think we should try to keep the number of places that
handle stack overflow in "special" ways as small as possible.
The problem with CommitTransactionCommand (or rather
AbortCurrentTransaction() which has the same problem)
and ShowTransactionStateRec is that they get called in a state where
aborting can lead to a panic. If you add a "check_stack_depth()" to them
and try to reproducer scripts that Egor posted, you still get a panic.
I'm not sure if MemoryContextStats() could safely elog(ERROR). But at
least it would mask the "out of memory" that caused the stats to be
printed in the first place.
--
Heikki Linnakangas
Neon (https://neon.tech)
