On 2025/05/02 14:58, torikoshia wrote:
I confirmed that with this patch applied, the process no longer crashes even 
after applying the change Robert used to trigger the crash.

a small nitpick:

+                * requested  repeatedly and rapidly, potentially leading to 
infinite

It looks like there are two spaces between "requested" and "repeatedly".

Thanks for the review and testing! I've fixed the issue you pointed out.
Updated patch attached.

Since git cherry-pick didn't work cleanly for v16 and earlier,
I've also prepared a separate patch for those versions.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From c39b3b83f8a04e781812bd3b83acb235babe3116 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fu...@postgresql.org>
Date: Sat, 3 May 2025 10:21:01 +0900
Subject: [PATCH v2] 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.

Reported-by: Robert Haas <robertmh...@gmail.com>
Author: Fujii Masao <masao.fu...@gmail.com>
Reviewed-by: Atsushi Torikoshi <torikos...@oss.nttdata.com>
Discussion: 
https://postgr.es/m/ca+tgmozmrv32tbnrrftvf9iwlntgqbhyslvcrhguwzvctph...@mail.gmail.com
Backpatch-through: 14
---
 src/backend/utils/mmgr/mcxt.c | 60 ++++++++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 19 deletions(-)

diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 9fc83f11f6f..73dcd64c351 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -1198,28 +1198,50 @@ HandleLogMemoryContextInterrupt(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 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, 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 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, false);
+       }
+       PG_FINALLY();
+       {
+               in_progress = false;
+       }
+       PG_END_TRY();
 }
 
 void *
-- 
2.49.0

From 35bb669bcf1be12d7517b3cc993bcf519c0db361 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fu...@postgresql.org>
Date: Sat, 3 May 2025 10:21:01 +0900
Subject: [PATCH v2] 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.

Reported-by: Robert Haas <robertmh...@gmail.com>
Author: Fujii Masao <masao.fu...@gmail.com>
Reviewed-by: Atsushi Torikoshi <torikos...@oss.nttdata.com>
Discussion: 
https://postgr.es/m/ca+tgmozmrv32tbnrrftvf9iwlntgqbhyslvcrhguwzvctph...@mail.gmail.com
Backpatch-through: 14
---
 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.c
index 7d28ca706eb..f0a8ccdfe12 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 via email to