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: pgsql-hackers@lists.postgresql.org
Date: 2021-03-17T04:01:55Z
Lists: pgsql-hackers 
"Andres Freund" <andres@anarazel.de> writes:
> On Tue, Mar 16, 2021, at 20:01, Tom Lane wrote:
>> 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.

OK, we're in violent agreement then -- I must've misread what you wrote.

>>> I think the run might have shown a genuine leak:

>> I didn't see anything like that after applying the fixes I showed before.

> 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 agree that having to reset CacheContext is not something we
should count as "still reachable", and I'm pretty sure that the
existing valgrind infrastructure doesn't count it that way.

As for the particular point about ParallelBlockTableScanWorkerData,
I agree with your question to David about why that's in TableScanDesc
not HeapScanDesc, but I can't get excited about it not being freed in
heap_endscan.  That's mainly because I do not believe that anything as
complex as a heap or indexscan should be counted on to be zero-leakage.
The right answer is to not do such operations in long-lived contexts.
So if we're running such a thing while switched into CacheContext,
*that* is the bug, not that heap_endscan didn't free this particular
allocation.

			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.