Horton

Re: Why our Valgrind reports suck

Yasir <yasir.hussain.shah@gmail.com>

View thread View on postgr.es
From: Yasir <yasir.hussain.shah@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>, Álvaro Herrera <alvherre@kurilemu.de>, pgsql-hackers@lists.postgresql.org
Date: 2025-05-22T15:30:34Z
Lists: pgsql-hackers 
On Wed, May 21, 2025 at 10:14 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Here's a v2 patchset that reaches the goal of zero reported leaks
> in the core regression tests, with some caveats:
>
> * Rather than completely fixing the function-cache and
> TS-dictionary-cache issues, I just added suppression rules to
> hide them.  I am not convinced it is worth working harder than that.
> The patchset does include some fixes that clean up low-hanging fruit
> in that area, but going further seems like a lot of work (and risk of
> bugs) for fairly minimal gain.  The core regression tests show less
> than 10K "suppressed" space in all test sessions but three, and those
> three are still under 100K.
>
> * The patch series assumes that the ModifyTable fix discussed at [1]
> is already applied.
>
> * I still observe leaks in ProcessGetMemoryContextInterrupt, but
> I think the consensus is we should just revert that as not yet ready
> for prime time [2].
>
> 0001 is the same as before except I did more work on the comments.
> I concluded that we were overloading the term "chunk" too much,
> so I invented the term "vchunk" for Valgrind's notion of chunks.
> (Feel free to suggest other terminology.)
>
> 0002 is new work to fix up MemoryContextAllocAligned so it doesn't
> cause possible-leak complaints.
>
>
I tested with the provided v2 patch series making sure mentioned [1]
applied.
More than 800 backend valgrind output files generated against regression,
among which 237 files contain suppressed: > 0 entries, of which 5 files
also contain "definitely lost: > 0 bytes entries".
The Maximum leak I found in these valgrind output files is 960 bytes only.

Whilst, the original issue I posted [link
<https://www.postgresql.org/message-id/flat/CAA9OW9e536dJanVKZRd_GKQ4wN_m5rhsMnrL6ZvaWagzLwv3%3Dg%40mail.gmail.com>]
is fixed. There are no leaks.

Regards,

Yasir Hussain
Data Bene


> The rest are more or less bite-sized fixes of individual problems.
> Probably we could squash a lot of them for final commit, but I
> thought it'd be easier to review like this.  Note that I'm not
> expecting 0013 to get applied in this form [3], but without it we
> have various gripes about memory leaked from plancache entries.
>
>                         regards, tom lane
>
> [1]
> https://www.postgresql.org/message-id/flat/213261.1747611093%40sss.pgh.pa.us
> [2]
> https://www.postgresql.org/message-id/594293.1747708165%40sss.pgh.pa.us
> [3]
> https://www.postgresql.org/message-id/605328.1747710381%40sss.pgh.pa.us
>
>

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.