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/