From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pgsql: Add function to log the memory contexts of specified backend pro
Date: Fri, 2 May 2025 09:02:43 +0900
Message-ID: <1983bca8-e1c4-45cd-9fd4-de38cd7b5896@oss.nttdata.com> (raw)
In-Reply-To: <b636ab38-8c55-4deb-a621-8651dcab4ced@oss.nttdata.com>
References: <E1lTdaH-0002Zo-DS@gemulon.postgresql.org>
<CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com>
<ce64a21b-9764-4782-ac94-58703ad6b258@oss.nttdata.com>
<CA+TgmoZfVES0SaS3NpP1kf__fW1ODc_wzVaNNC9HBHvERvh9xA@mail.gmail.com>
<b636ab38-8c55-4deb-a621-8651dcab4ced@oss.nttdata.com>
On 2025/05/02 2:27, Fujii Masao wrote:
>
>
> On 2025/05/01 21:42, Robert Haas wrote:
>> On Thu, May 1, 2025 at 3:53 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>> Just idea, what do you think about adding a flag to indicate whether
>>> ProcessLogMemoryContextInterrupt() is currently running? Then,
>>> when a backend receives a signal and ProcessLogMemoryContextInterrupt()
>>> is invoked, it can simply return immediately if the flag is already set
>>> like this:
>>
>> I think that something like this could work, but you would need more
>> than this. Otherwise, if the function errors out, the flag would
>> remain permanently set.
>
> Yes, we need to either use PG_TRY()/PG_FINALLY() or handle the flag as
> a global variable and reset it in the error handling path. I think using
> PG_TRY()/PG_FINALLY() is the simpler option.
I've implemented the patch in that way. Patch attached.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:v1-0001-Add-guard-to-prevent-recursive-memory-contexts-lo.patch (text/plain, 3.6K) download
From 5a65280d10fec6e4c5577e26eeddeb10c9ddb13f Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Fri, 2 May 2025 00:35:48 +0900
Subject: [PATCH v1] Add guard to prevent recursive memory contexts logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory contexts logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory contexts logging was introduced.
---
src/backend/utils/mmgr/mcxt.c | 61 +++++++++++++++++++++++------------
1 file changed, 41 insertions(+), 20 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.cindex 7d28ca706eb..907c7f7affe 100644--- a/src/backend/utils/mmgr/mcxt.c+++ b/src/backend/utils/mmgr/mcxt.c@@ -1383,29 +1383,50 @@ HandleGetMemoryContextInterrupt(void)
void
ProcessLogMemoryContextInterrupt(void)
{
+ static bool in_progress = false;+
LogMemoryContextPending = false;
- /*- * Use LOG_SERVER_ONLY to prevent this message from being sent to the- * connected client.- */- ereport(LOG_SERVER_ONLY,- (errhidestmt(true),- errhidecontext(true),- errmsg("logging memory contexts of PID %d", MyProcPid)));+ PG_TRY();+ {+ /*+ * Exit immediately if memory contexts logging is already in progress.+ * This prevents recursive calls, which could occur if logging is+ * requested repeatedly and rapidly, potentially leading to infinite+ * recursion and a crash.+ */+ if (in_progress)+ return;+ in_progress = true;- /*- * When a backend process is consuming huge memory, logging all its memory- * contexts might overrun available disk space. To prevent this, we limit- * the depth of the hierarchy, as well as the number of child contexts to- * log per parent to 100.- *- * As with MemoryContextStats(), we suppose that practical cases where the- * dump gets long will typically be huge numbers of siblings under the- * same parent context; while the additional debugging value from seeing- * details about individual siblings beyond 100 will not be large.- */- MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);+ /*+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the+ * connected client.+ */+ ereport(LOG_SERVER_ONLY,+ (errhidestmt(true),+ errhidecontext(true),+ errmsg("logging memory contexts of PID %d", MyProcPid)));++ /*+ * When a backend process is consuming huge memory, logging all its+ * memory contexts might overrun available disk space. To prevent+ * this, we limit the depth of the hierarchy, as well as the number of+ * child contexts to log per parent to 100.+ *+ * As with MemoryContextStats(), we suppose that practical cases where+ * the dump gets long will typically be huge numbers of siblings under+ * the same parent context; while the additional debugging value from+ * seeing details about individual siblings beyond 100 will not be+ * large.+ */+ MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);+ }+ PG_FINALLY();+ {+ in_progress = false;+ }+ PG_END_TRY();
}
/*
--
2.49.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, robertmhaas@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Add function to log the memory contexts of specified backend pro
In-Reply-To: <1983bca8-e1c4-45cd-9fd4-de38cd7b5896@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox