On Tue, 30 Jun 2026 at 06:56, Thom Brown <[email protected]> wrote:
>
> On Tue, 30 Jun 2026 at 03:17, Thomas Munro <[email protected]> wrote:
> >
> > On Tue, Jun 30, 2026 at 1:21 PM Thom Brown <[email protected]> wrote:
> > > On Tue, 30 Jun 2026 at 00:24, Thomas Munro <[email protected]> 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).
>
> AFAICT, ownership transfers to the caller on success, and the lazy
> clearing at next statement is just timing. But I think
> ECPGfree_auto_mem() isn't only internal failure-cleanup. It's exported
> and called by application code as the bulk-free for GET DESCRIPTOR ...
> INTO :array results, which the caller does own.
>
> I'm looking at dynalloc.pgc, line 83, and those arrays look like
> they're auto-allocated in descriptor.c:
>
> Line 437 onward, for the data array:
>
> /* allocate storage if needed */
> if (arrsize == 0 && *(void **) var == NULL)
> {
>         void       *mem = ecpg_auto_alloc(offset * ntuples, lineno);
>
>         if (!mem)
>         {
>                 va_end(args);
>                 return false;
>         }
>         *(void **) var = mem;
>         var = mem;
> }
>
> And there's another one at line 557 onward for the indicator array

Correction: the allocation I pointed at in descriptor.c (line 437)
isn't for the data array. That branch handles
RETURNED_LENGTH/RETURNED_OCTET_LENGTH items. The data and indicator
arrays in dynalloc.pgc actually come from ecpg_store_result(),
execute.c lines 398 and 409, which ECPGget_desc() calls at
descriptor.c line 519. The one at line 557 is for an indicator given
without a data variable.

Doesn't change the conclusion: everything GET DESCRIPTOR materialises
into a zero-sized pointer variable ends up on the auto-mem list, so
the bulk-free usage and the need for thread-exit node cleanup still
stand.

Thom


Reply via email to