Re: Why our Valgrind reports suck

On Wed, May 21, 2025 at 10:14 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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
>
>