Re: Avoiding memory leak when compilation of a function fails

út 27. 5. 2025 v 7:27 odesílatel Amul Sul <sulamul(at)gmail(dot)com> napsal:

> On Tue, May 27, 2025 at 4:23 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Back in [1], Andres complained that repeated attempts to create
> > an invalid plpgsql function (one that fails initial compilation)
> > leak memory, for example
> >
> > DO $do$
> > BEGIN
> > FOR i IN 1 .. 100000 LOOP
> > BEGIN
> > CREATE OR REPLACE FUNCTION foo() RETURNS VOID
> > LANGUAGE plpgsql AS $f$BEGIN frakbar; END;$f$;
> > EXCEPTION WHEN others THEN
> > END;
> > END LOOP;
> > END;$do$;
> >
> > The reason is that we create the long-lived function cache context
> > and detect the syntax error only while trying to fill it in.
> > As I remarked at the time, we could make that better by making
> > the cache context initially short-lived and reparenting it only
> > after it's known good. The attached patch does that.
> >
> > I noted that the CachedFunction struct made by funccache.c gets
> > leaked too. (That's not new, but the blame used to fall on plpgsql's
> > equivalent of that code.) That's not hard to fix in typical cases,
> > at the price of an extra PG_TRY, which seems okay in a code path that
> > is setting up a long-lived cache entry. Also done in the attached.
> >
> > I thought that SQL-language functions might have this issue too,
> > but they do not, because sql_compile_callback already uses the
> > reparenting trick. (I followed its lead in making the function
> > contexts live under CacheMemoryContext not TopMemoryContext.)
> >
> > If you run the above example long enough, you will also observe a
> > slow leak in TopTransactionContext. AFAICT that is from accumulating
> > invalidation messages from the failed pg_proc insertions, so it's not
> > specific to functions but applies to any DDL in a loop. Fixing that
> > seems outside the scope of this patch.
> >
> > I think this is a bug fix, so I'm inclined to squeeze it into v18.
> > I am not sure if it's worth developing a back-patchable version.
> > The pl_comp.c change probably applies easily further back, and
> > would be enough to get the bulk of the benefit.
> >
> > Comments?
> >
>
> The patch seems reasonable and the changes appear straightforward
> enough for a backport. However, I am not sure about the backporting,
> as the leak doesn't seem to occur very frequently.
>

+1

Regards

Pavel

>
> Regards,
> Amul
>
>
>