Re: Stack overflow issue

From: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
To: Egor Chindyaskin <kyzevan23(at)mail(dot)ru>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Sascha Kuhl <yogidabanli(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Stack overflow issue
Date: 2024-03-07 09:07:34
Message-ID: CAPpHfdt-vAppuNxFvhEWNLk+8GpXm4rgPJWxqR8FK2g6GaZMhQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi, Egor!

On Thu, Mar 7, 2024 at 9:53 AM Egor Chindyaskin <kyzevan23(at)mail(dot)ru> wrote:
>
> > 6 march 2024 г., at 19:17, Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
> >
> > The revised set of remaining patches is attached.
> >
> > 0001 Turn tail recursion into iteration in CommitTransactionCommand()
> > I did minor revision of comments and code blocks order to improve the
> > readability.
> >
> > 0002 Avoid stack overflow in ShowTransactionStateRec()
> > I didn't notice any issues, leave this piece as is.
> >
> > 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.
> >
> > Generally, I think this patchset fixes important stack overflow holes.
> > It is quite straightforward, clear and the code has a good shape. I'm
> > going to push this if no objections.
>
> I have tested the scripts from message [1]. After applying these patches and Tom Lane’s patch from message [2], all of the above mentioned functions no longer caused the server to crash. I also tried increasing the values in the presented scripts, which also did not lead to server crashes. Thank you!
> Also, I would like to clarify something. Will fixes from message [3] and others be backported to all other branches, not just the master branch? As far as I remember, Tom Lane made corrections to all branches. For example [4].
>
> Links:
> 1. https://www.postgresql.org/message-id/343ff14f-3060-4f88-9cc6-efdb390185df%40mail.ru
> 2. https://www.postgresql.org/message-id/386032.1709765547%40sss.pgh.pa.us
> 3. https://www.postgresql.org/message-id/CAPpHfduZqAjF%2B7rDRP-RGNHjOXy7nvFROQ0MGS436f8FPY5DpQ%40mail.gmail.com
> 4. https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=e07ebd4b

Thank you for your feedback!

Initially I didn't intend to backpatch any of these. But on second
thought with the references you provided, I think we should backpatch
simple check_stack_depth() checks from d57b7cc333 to all supported
branches, but apply refactoring of memory contextes and transaction
commit/abort just to master. Opinions?

------
Regards,
Alexander Korotkov

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2024-03-07 09:36:45 Re: speed up a logical replica setup
Previous Message John Naylor 2024-03-07 08:18:11 Re: [PoC] Improve dead tuple storage for lazy vacuum