Horton

Re: Getting better results from valgrind leak tracking

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: David Rowley <dgrowleyml@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2021-03-17T03:01:47Z
Lists: pgsql-hackers 
Andres Freund <andres@anarazel.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 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'm not trying to claim there are no leaks anywhere, just that the amount
of noise from those issues swamps all the real problems.  I particularly
recommend not believing anything related to catcache or relcache if you
haven't applied that admittedly-hacky patch.

> Hm. For me the number of leaks seem to stay the same with or without
> your changes related to this. Is this a USE_VALGRIND build?

Yeah ...

> 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.
We could chase that sort of thing after we clean up the other problems.

			regards, tom lane

Commits

  1. Improve our support for Valgrind's leak tracking.

  2. Increment xactCompletionCount during subtransaction abort.

  3. Adjust design of per-worker parallel seqscan data struct

  4. Don't leak compiled regex(es) when an ispell cache entry is dropped.

  5. Don't leak malloc'd error string in libpqrcv_check_conninfo().

  6. Don't leak malloc'd strings when a GUC setting is rejected.

  7. Don't leak rd_statlist when a relcache entry is dropped.

  8. Don't run RelationInitTableAccessMethod in a long-lived context.