Horton

Re: Stack overflow issue

Alexander Korotkov <aekorotkov@gmail.com>

View thread View on postgr.es
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, Egor Chindyaskin <kyzevan23@mail.ru>, Sascha Kuhl <yogidabanli@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-03-06T23:24:33Z
Lists: pgsql-hackers 
On Thu, Mar 7, 2024 at 12:52 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > The revised set of remaining patches is attached.
> > ...
> > 0003 Avoid recursion in MemoryContext functions
> > I've renamed MemoryContextTraverse() => MemoryContextTraverseNext(),
> > which I think is a bit more intuitive.  Also I fixed
> > MemoryContextMemConsumed(), which was still trying to use the removed
> > argument "print" of MemoryContextStatsInternal() function.
>
> This patch still doesn't compile for me --- MemoryContextMemConsumed
> got modified some more by commit 743112a2e, and needs minor fixes.
>
> I initially didn't like the definition of MemoryContextTraverseNext
> because it requires two copies of the "process node" logic.  However,
> that seems fine for most of the callers, and even where we are
> duplicating logic it's just a line or so, so I guess it's ok.
> However, MemoryContextTraverseNext seems undercommented to me, plus
> the claim that it traverses in depth-first order is just wrong.
>
> I found some bugs in MemoryContextStatsInternal too: the old
> logic assumed that ichild exceeding max_children was the only
> way to get into the summarization logic, but now ichild minus
> max_children could very well be negative.  Fortunately we can
> just reset ichild to zero and not worry about having any
> connection between the first loop and the second.
>
> Here's a v5 of 0003 with those issues and some more-cosmetic ones
> cleaned up.  I didn't look at 0001 or 0002.
>

Tom, thank you for your revision of this patch!

Sorry for tediousness, but isn't pre-order a variation of depth-first order
[1]?

Links.
1. https://en.wikipedia.org/wiki/Tree_traversal#Depth-first_search

------
Regards,
Alexander Korotkov

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.