Re: Stack overflow issue

On 11/01/2024 19:37, Robert Haas wrote:
> On Wed, Jan 10, 2024 at 4:25 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> 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.
> 
> Hmm, that's unfortunate. I'm not sure what to do about that. But I'd
> still suggest looking for a goto-free approach.

Here's one goto-free attempt. It adds a local loop to where the 
recursion was, so that if you have a chain of subtransactions that need 
to be aborted in CommitTransactionCommand, they are aborted iteratively. 
The TBLOCK_SUBCOMMIT case already had such a loop.

I added a couple of comments in the patch marked with "REVIEWER NOTE", 
to explain why I changed some things. They are to be removed before 
committing.

I'm not sure if this is better than a goto. In fact, even if we commit 
this, I think I'd still prefer to replace the remaining recursive calls 
with a goto. Recursion feels a weird to me, when we're unwinding the 
states from the stack as we go.

Of course we could use a "for (;;) { ... continue }" construct around 
the whole function, instead of a goto, but I don't think that's better 
than a goto in this case.

-- 
Heikki Linnakangas
Neon (https://neon.tech)