Thread

Re: lsyscache: free IndexAmRoutine objects returned by GetIndexAmRoutineByAmId()

Álvaro Herrera <alvherre@kurilemu.de> — 2025-12-30T14:15:26Z

On 2025-Dec-29, Tom Lane wrote:

> The main objection that could be raised to this is that the old coding
> ensures that any memory leaked during GetIndexAmRoutine() will be
> leaked in the expected-to-be-short-lived caller's context, but now
> it'd be leaked in the cache's rd_indexcxt.

One thing we can perhaps do is (in assert-enabled builds) to detect
whether memory usage for that context has increased during
InitIndexAmRoutine and raise a warning if so.  Then extension authors
would realize this and have a chance to fix it promptly.

I tried this out and it doesn't work as well as I thought, due to how
AllocSet works: we don't get a difference in the amount of allocated
memory (thus no WARNING) unless we add some padding bytes to
IndexAmRoutine, as shown below (of course, just POC-quality -- didn't
kry to compile without assertions).  Given this, I'm not terribly
optimistic about this idea.  I tested this by adding
palloc(sizeof(IndexAmRoutine)) in brinhandler() and verifying that a few
regression tests fail with the added warning message.


diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 88259f7c228..75f2a2f5e37 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1421,6 +1421,7 @@ static void
 InitIndexAmRoutine(Relation relation)
 {
 	MemoryContext oldctx;
+	Size	allocated	PG_USED_FOR_ASSERTS_ONLY;
 
 	/*
 	 * We formerly specified that the amhandler should return a palloc'd
@@ -1429,7 +1430,19 @@ InitIndexAmRoutine(Relation relation)
 	 * the amhandler in the relation's rd_indexcxt.
 	 */
 	oldctx = MemoryContextSwitchTo(relation->rd_indexcxt);
+
+#ifdef USE_ASSERT_CHECKING
+	allocated = MemoryContextMemAllocated(CurrentMemoryContext, false);
+#endif
+
 	relation->rd_indam = GetIndexAmRoutine(relation->rd_amhandler);
+
+#ifdef USE_ASSERT_CHECKING
+	if (MemoryContextMemAllocated(CurrentMemoryContext, false) - allocated != 0)
+		elog(WARNING, "memory allocated while initializing access method %u (was %zu, now %zu)",
+			 relation->rd_amhandler, allocated, MemoryContextMemAllocated(CurrentMemoryContext, false));
+#endif
+
 	MemoryContextSwitchTo(oldctx);
 }
 
diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h
index 278da36bc08..78fcbcffc14 100644
--- a/src/include/access/amapi.h
+++ b/src/include/access/amapi.h
@@ -322,6 +322,11 @@ typedef struct IndexAmRoutine
 	/* interface functions to support planning */
 	amtranslate_strategy_function amtranslatestrategy;	/* can be NULL */
 	amtranslate_cmptype_function amtranslatecmptype;	/* can be NULL */
+
+#ifdef USE_ASSERT_CHECKING
+	char		pad[512];
+#endif
+
 } IndexAmRoutine;

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/