On Sun, Jul 21, 2013 at 12:40:38PM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > Reasonable enough. Code that does use subtransactions will need to be more
> > careful than before to manually free tuple tables in the non-error case.
> > Failure to do so has been creating a leak that lasts until SPI_finish(), but
> > it will now be able to cause a transaction-lifespan leak.
>
> Hmm ... good point. The other plan I'd been considering was to add
> explicit tracking inside spi.c of all tuple tables created within the
> current procedure, and then have AtEOSubXact_SPI flush any that were
> created inside a failed subxact. The tables would still be children of
> the procCxt and thus could not be leaked past SPI_finish. When you
> suggested attaching to subtransaction contexts I thought that would let
> us get away without this additional bookkeeping logic, but maybe we
> should bite the bullet and add the extra logic.
Is there reason to believe we wouldn't eventually find a half dozen other
allocations calling for similar bespoke treatment? Does something make tuple
tables special among memory allocations, or are they just the garden-variety
allocation that happens to bother the test case at hand?
> A change that's meant
> to remove leak risks really shouldn't be introducing other, new leak
> risks.
Yes; that gives one pause.
Thanks,
nm
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com