| From: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
|---|---|
| To: | Thom Brown <thom(at)linux(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Per-thread leak in ECPG's memory.c |
| Date: | 2026-06-30 02:16:47 |
| Message-ID: | CA+hUKGKk51hhwYPPxeeDBjTy-TD2o8p-OQvQ2Ov3qSSuix=fNA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Jun 30, 2026 at 1:21 PM Thom Brown <thom(at)linux(dot)com> wrote:
> On Tue, 30 Jun 2026 at 00:24, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> > EXEC SQL AT con1 SELECT datname INTO :anything FROM pg_database;
> > EXEC SQL AT con2 ... something that reaches ecpg_raise() ...
> >
> > /* Why should "anything" not be accessible, and mine to free(), here? */
>
> I see, yeah, that does seem wrong. So would it just be a matter of
> moving ecpg_clear_auto_mem() out of ecpg_do_prologue() and put it into
> ecpg_do_epilogue() so that it cleans up at the end of the statement
> instead of the start of the next one?
Oh, that sounds plausible and simple! Have I understood the
lifetime/ownership rule correctly here?: after a successful statement,
ownership of auto-allocated objects is transferred to the caller, and
the fact that we currently don't "clear" (forget, disown) the list
until the next statement is just an implementation detail and we could
indeed make it "eager". In contrast, ECPGfree_auto_mem() is only used
to clean up partially allocated variables on failure, since then the
caller doesn't own anything. If that all makes sense, then perhaps
the auto-mem list could always be NULL between ECPG statements,
meaning that ownership of "anything" would already have been
transferred to the caller (cleared) in this example. And then perhaps
we wouldn't even need a thread-exit destructor here? On the other
hand, that'd be a subtle semantic change that could upset an
application. So perhaps this would have to be a well-signalled
master-only change, if it makes sense to do it? I haven't actually
tried any of that or written a test (feel free if you're interested in
looking at this), I'm just trying to confirm my understanding of the
design.
This is independent of the destructor issue I started with, which I
assume we'd want to back-patch. It would be nice if we could do
something like the above in master and that leads to deleting the
destructor though (we still need thread-exit cleanup for other ECPG
things though, and need a new system for that that also works on
Windows, more patches soon..., so it's not a big deal either way).
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Xuneng Zhou | 2026-06-30 02:25:48 | Re: 048_vacuum_horizon_floor.pl hangs due to wakeup lost inside LockBufferForCleanup |
| Previous Message | Peter Smith | 2026-06-30 01:55:37 | Re: Proposal: Conflict log history table for Logical Replication |