From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: memory leak in trigger handling (since PG12) |
Date: | 2023-05-23 16:39:40 |
Message-ID: | 960113.1684859980@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> writes:
> it seems there's a fairly annoying memory leak in trigger code,
> introduced by
> ...
> Attached is a patch, restoring the pre-12 behavior for me.
> While looking for other places allocating stuff in ExecutorState (for
> the UPDATE case) and leaving it there, I found two more cases:
> 1) copy_plpgsql_datums
> 2) make_expanded_record_from_tupdesc
> make_expanded_record_from_exprecord
> All of this is calls from plpgsql_exec_trigger.
Not sure about the expanded-record case, but both of your other two
fixes feel like poor substitutes for pushing the memory into a
shorter-lived context. In particular I'm quite surprised that
plpgsql isn't already allocating that workspace in the "procedure"
memory context.
> I wonder how much we should care about these cases. On the one hand we
> often leave the cleanup up to the memory context, but the assumption is
> the context is not unnecessarily long-lived. And ExecutorState is not
> that. And leaking memory per-row does not seem great either.
I agree per-row leaks in the ExecutorState context are not cool.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Markus Winand | 2023-05-23 17:12:23 | Wrong command name in writeable-CTE related error messages |
Previous Message | Robert Haas | 2023-05-23 16:33:50 | Re: RFI: Extending the TOAST Pointer |