From: Tom Lane <tgl@sss.pgh.pa.us>
To: exclusion@gmail.com
cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18374: Printing memory contexts on OOM condition might lead to segmentation fault
In-reply-to: <18374-ebb8113ce4d02f0d@postgresql.org>
References: <18374-ebb8113ce4d02f0d@postgresql.org>
Comments: In-reply-to PG Bug reporting form <noreply@postgresql.org>
	message dated "Sat, 02 Mar 2024 06:00:01 +0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3120720.1709395887.1@sss.pgh.pa.us>
Date: Sat, 02 Mar 2024 11:11:27 -0500
Message-ID: <3120721.1709395887@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/3120721.1709395887%40sss.pgh.pa.us>
Precedence: bulk

PG Bug reporting form <noreply@postgresql.org> writes:
> When a backend with deeply nested memory contexts hits out-of-memory
> condition and logs the contexts, it might lead to a segmentation fault
> (due to the lack of free memory again).

Hmph.  That's not an out-of-memory crash, that's a stack-too-deep
crash.

Seems like we ought to do one or both of these:

1. Put a CHECK_STACK_DEPTH() call in MemoryContextStatsInternal.

2. Teach MemoryContextStatsInternal to refuse to recurse more
than N levels, for N perhaps around 100.

Neither of these are very attractive though, as they'd obscure
the OOM situation that we're trying to help debug.

It strikes me that we don't actually need recursion in order to
traverse the context tree: since the nodes have parent pointers,
it'd be possible to visit them all using only iteration.  The
recursion seems necessary though to manage the child summarization
logic as we have it (in particular, we must have a local_totals
per level to produce summarization like this).  Maybe we could
modify solution #2 into

2a. Once we get more than say 100 levels deep, summarize everything
below that in a single line, obtained in an iterative rather than
recursive traversal.

I wonder whether MemoryContextDelete and other cleanup methods
also need to be rewritten to avoid recursion.  In the infinite_recurse
test case I think we escape trouble because we longjmp out of most
of the stack before we try to clean up --- but you could probably
devise a test case that tries to do a subtransaction abort at a
deep call level, and then maybe kaboom?

			regards, tom lane