Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix memory leakage when function compilation fails.

  1. Avoiding memory leak when compilation of a function fails

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-05-26T22:53:24Z

    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?

			regards, tom lane

[1] https://www.postgresql.org/message-id/20210317055718.v6qs3ltzrformqoa@alap3.anarazel.de

  2. Re: Avoiding memory leak when compilation of a function fails

    amul sul <sulamul@gmail.com> — 2025-05-27T05:26:18Z

    On Tue, May 27, 2025 at 4:23 AM Tom Lane <tgl@sss.pgh.pa.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.

Regards,
Amul

  3. Re: Avoiding memory leak when compilation of a function fails

    Pavel Stehule <pavel.stehule@gmail.com> — 2025-05-27T05:30:00Z

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

> On Tue, May 27, 2025 at 4:23 AM Tom Lane <tgl@sss.pgh.pa.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
>
>
>