Horton

Re: Why our Valgrind reports suck

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Álvaro Herrera <alvherre@kurilemu.de>, pgsql-hackers@lists.postgresql.org
Date: 2025-05-23T01:19:00Z
Lists: pgsql-hackers 
Hi,

0001:

This is rather wild, I really have only the dimmest memory of that other
thread even though I apparently resorted to reading valgrind's source code...

I think the vchunk/vpool naming, while not necessarily elegant, is helpful.


0002: Makes sense.


0003:
0004:
0005:

Ugh, that's rather gross. Not that I have a better idea. So it's probably
worth doing ...


0006: LGTM


0007:

+	/* Run the rest in xact context to avoid Valgrind leak complaints */
+	MemoryContextSwitchTo(TopTransactionContext);
 
It seems like it also protects at least somewhat against actual leaks?


0008: LGTM


0009:

Seems like we really ought to do more here. But it's a substantial
improvement, so let's not let perfect be the enemy of good.


0010:
0011:

Not great, but better than not doing anything.


0012:

Hm. Kinda feels like we should just always do it the USE_VALGRIND
approach. ISTM that if domain typecache entry building is a bottleneck, there
are way bigger problems than a copyObject()...


0014:

I guess I personally would try to put the freeing code into a helper, but
it's a close call.


0015:

I'd probably put the list_free() after the
LWLockRelease(LogicalRepWorkerLock)?


0016:

Agreed with the concern stated in the commit message, but addressing that
would obviously be a bigger project.


0017:

A tad weird to leave the comments above the removed = NILs in place, even
though it's obviously still correct.


0018: LGTM.


> But this is the last step to get to zero reported leaks in a run of the core
> regression tests, so let's do it.

I assume that's just about the core tests, not more? I.e. I can't make skink
enable leak checking?

Greetings,

Andres Freund

Commits

  1. Undo thinko in commit e78d1d6d4.

  2. Avoid leakage of zero-length arrays in partition_bounds_copy().

  3. Fix MemoryContextAllocAligned's interaction with Valgrind.

  4. Fix assorted pretty-trivial memory leaks in the backend.

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

  6. Reduce leakage during PL/pgSQL function compilation.

  7. Silence Valgrind leakage complaints in more-or-less-hackish ways.

  8. Silence complaints about leaks in PlanCacheComputeResultDesc.

  9. Suppress complaints about leaks in TS dictionary loading.

  10. Suppress complaints about leaks in function cache loading.

  11. Fix per-relation memory leakage in autovacuum.

  12. Fix AlignedAllocRealloc to cope sanely with OOM.