Re: Memory leak in plpython3u (with testcase and patch)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Mat Arye <mat(at)timescale(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Memory leak in plpython3u (with testcase and patch)
Date: 2025-01-10 22:45:22
Message-ID: 2709062.1736549122@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> However, I don't really see why we need to use that scratch context.
> PLy_spi_execute_plan runs a subtransaction, so we could perfectly well
> use the subtransaction's CurTransactionContext.

Nah, I'm wrong about that: I forgot CurTransactionContext goes away on
subtransaction abort, but not subtransaction commit. So we really
need to make our own context with suitable lifespan.

I also noticed that the code is moving heaven and earth to store
the argument Datums ... but for some reason not the associated
isnull flags ... in the PLyPlanObject. This is just nuts, because
the values don't need to survive past the functions that are computing
them. In the cursor case, the values will be copied into the cursor
portal, so they still don't need to survive. We can drop all of
that.

So I arrive at the attached. (This is just for HEAD. In the back
branches, we'd better leave the PLyPlanObject.values field in place
for ABI safety, but we don't have to use it.)

regards, tom lane

Attachment Content-Type Size
v2-fix-plpython-memory-leaks.patch text/x-diff 6.6 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2025-01-10 22:59:51 Re: Moving the vacuum GUCs' docs out of the Client Connection Defaults section
Previous Message Andrew Kane 2025-01-10 22:26:50 Restore support for USE_ASSERT_CHECKING in extensions only