From: Tom Lane <tgl@sss.pgh.pa.us>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
cc: Andres Freund <andres@anarazel.de>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: memory leak in trigger handling (since PG12)
In-reply-to: <b9f40ba0-6b8b-0a4a-ad10-891e6f41d882@enterprisedb.com>
References: <222a3442-7f7d-246c-ed9b-a76209d19239@enterprisedb.com> <20230523171433.earidmyzock7fnk4@awork3.anarazel.de> <b9f40ba0-6b8b-0a4a-ad10-891e6f41d882@enterprisedb.com>
Comments: In-reply-to Tomas Vondra <tomas.vondra@enterprisedb.com>
	message dated "Tue, 23 May 2023 23:26:42 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <993813.1684877984.1@sss.pgh.pa.us>
Date: Tue, 23 May 2023 17:39:44 -0400
Message-ID: <993814.1684877984@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/993814.1684877984%40sss.pgh.pa.us>
Precedence: bulk

Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
> The really hard thing was determining what causes the memory leak - the
> simple instrumentation doesn't help with that at all. It tells you there
> might be a leak, but you don't know where did the allocations came from.

> What I ended up doing is a simple gdb script that sets breakpoints on
> all palloc/pfree variants, and prints info (including the backtrace) for
> each call on ExecutorState. And then a script that aggregate those to
> identify which backtraces allocated most chunks that were not freed.

FWIW, I've had some success localizing palloc memory leaks with valgrind's
leak detection mode.  The trick is to ask it for a report before the
context gets destroyed.  Beats writing your own infrastructure ...

> Would require testing with more data, though. I doubt we'd find much
> with our regression tests, which have tiny data sets.

Yeah, it's not clear whether we could make the still-hypothetical check
sensitive enough to find leaks using small test cases without getting an
unworkable number of false positives.  Still, might be worth trying.
It might be an acceptable tradeoff to have stricter rules for what can
be allocated in ExecutorState in order to make this sort of problem
more easily detectable.

			regards, tom lane