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

On Tue, 30 Dec 2025 at 15:15, Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
> 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

Hmm, wouldn't we be able to detect changes in
MemoryContextMemConsumed(ctx, counters) with one before and one after
GetIndexAmRoutine(), such as included below?

It would cause false positives if amroutine() does memory-related work
other than just returning an appropriate IndexAmRoutine pointer (if it
does so without switching to its own MemoryContext), but I don't think
that such false positives here are necessarily bad - the AM shouldn't
be doing much at this point in the code.

Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)

diff --git a/src/backend/utils/cache/relcache.c
b/src/backend/utils/cache/relcache.c
index 86b90765433..c17f9c3e53a 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1421,6 +1421,13 @@ static void
InitIndexAmRoutine(Relation relation)
{
MemoryContext oldctx;
+#if USE_ASSERT_CHECKING
+ Size prevusedmem;
+ MemoryContextCounters usage;
+
+ MemoryContextMemConsumed(relation->rd_indexcxt, &usage);
+ prevusedmem = usage.totalspace - usage.freespace;
+#endif

/*
* We formerly specified that the amhandler should return a
@@ -1431,6 +1438,12 @@ InitIndexAmRoutine(Relation relation)
oldctx = MemoryContextSwitchTo(relation->rd_indexcxt);
relation->rd_indam = GetIndexAmRoutine(relation->rd_amhandler);
MemoryContextSwitchTo(oldctx);
+
+#if USE_ASSERT_CHECKING
+ /* ensure the index routine was not palloc'd */
+ MemoryContextMemConsumed(relation->rd_indexcxt, &usage);
+ Assert(prevusedmem == (usage.totalspace - usage.freespace));
+#endif
}

/*
```