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

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/