Re: Getting better results from valgrind leak tracking

Hi,

On Tue, Mar 16, 2021, at 20:01, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2021-03-16 19:36:10 -0400, Tom Lane wrote:
> >> It doesn't appear to me that we leak much at all, at least not if you
> >> are willing to take "still reachable" blocks as not-leaked.
>
> > Well, I think for any sort of automated testing - which I think would be
> > useful - we'd really need *no* leaks.
>
> That seems both unnecessary and impractical. We have to consider that
> everything-still-reachable is an OK final state.

I don't consider "still reachable" a leak. Just definitely unreachable. And with a few tweaks that seems like we could achieve that?

> > I think the run might have shown a genuine leak:
>
> > ==2048803== 16 bytes in 1 blocks are definitely lost in loss record 139 of 906
> > ==2048803== at 0x89D2EA: palloc (mcxt.c:975)
> > ==2048803== by 0x2392D3: heap_beginscan (heapam.c:1198)
> > ==2048803== by 0x264E8F: table_beginscan_strat (tableam.h:918)
> > ==2048803== by 0x265994: systable_beginscan (genam.c:453)
> > ==2048803== by 0x83C2D1: SearchCatCacheMiss (catcache.c:1359)
> > ==2048803== by 0x83C197: SearchCatCacheInternal (catcache.c:1299)
>
> I didn't see anything like that after applying the fixes I showed before.
> There are a LOT of false positives from the fact that with our HEAD
> code, valgrind believes that everything in the catalog caches and
> most things in dynahash tables (including the relcache) are unreachable.

I think it's actually unreachable memory (unless you count resetting the cache context), based on manually tracing the code... I'll try to repro.

> > I do see a bunch of leaks bleats below fun:plpgsql_compile that I don't
> > yet understand. E.g.
>
> Those are probably a variant of what you were suggesting above, ie
> plpgsql isn't terribly careful not to leak random stuff while building
> a long-lived function parse tree. It's supposed to use a temp context
> for anything that might leak, but I suspect it's not thorough about it.

What I meant was that I didn't understand how there's not a leak danger when compilation fails halfway through, given that the context in question is below TopMemoryContext and that I didn't see a relevant TRY block. But that probably there is something cleaning it up that I didn't see.

Andres