Horton

Re: Stack overflow issue

Heikki Linnakangas <hlinnaka@iki.fi>

View thread View on postgr.es
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Egor Chindyaskin <kyzevan23@mail.ru>, Sascha Kuhl <yogidabanli@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-01-10T21:25:42Z
Lists: pgsql-hackers 
On 05/01/2024 19:23, Robert Haas wrote:
> On Fri, Nov 24, 2023 at 10:47 AM Heikki Linnakangas <hlinnaka@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)

Commits

  1. Refactoring for CommitTransactionCommand()/AbortCurrentTransaction()

  2. Avoid recursion in MemoryContext functions

  3. Avoid stack overflow in ShowTransactionStateRec()

  4. Turn tail recursion into iteration in CommitTransactionCommand()

  5. Adjust memory allocation functions to allow sibling calls

  6. Add missing check_stack_depth() to some recursive functions

  7. In the Snowball dictionary, don't try to stem excessively-long words.

  8. Defend against stack overrun in a few more places.