Обсуждение: memory leak checking

Hi,

On 2019-04-22 16:50:25 -0700, Mikhail Bautin wrote:
> What is the standard memory leak checking policy for the PostgreSQL
> codebase? I know there is some support for valgrind -- is the test suite
> being run continuously with valgrind on the build farm?

There's continuous use of valgrind on the buildfarm - but those animals
have leak checking disabled. Postgres for nearly all server allocations
uses memory contexts, which allows to bulk-free memory. There's also
plenty memory that's intentionally allocated till the end of the backend
lifetime (e.g. the various caches over the system catalogs).  Due to
that checks like valgrinds are not particularly meaningful.


> Is there any plan to support clang's AddressSanitizer?

Not for the leak portion. I use asan against the backend, after
disabling the leak check, and that's useful. Should probably set up a
proper buildfarm animal for that.


> I've seen a thread that memory leaks are allowed in initdb, because it is a
> short-lived process. Obviously they are not allowed in the database server.
> Are memory leaks allowed in the psql tool?

Leaks are allowed if they are once-per-backend type things. There's no
point in e.g. freeing information for timezone metadata, given that
it'll be used for the whole server lifetime. And there's such things in
psql too, IIRC.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2019-04-22 16:50:25 -0700, Mikhail Bautin wrote:
>> What is the standard memory leak checking policy for the PostgreSQL
>> codebase? I know there is some support for valgrind -- is the test suite
>> being run continuously with valgrind on the build farm?

> Leaks are allowed if they are once-per-backend type things. There's no
> point in e.g. freeing information for timezone metadata, given that
> it'll be used for the whole server lifetime. And there's such things in
> psql too, IIRC.

I would not call the timezone data a "leak", since it's still useful, and
accessible from static pointers, right up to exit.  A true leak for this
purpose is memory that's allocated but not usefully accessible, and I'd
say we discourage that; though small one-time leaks may not be worth the
trouble to get rid of.

            regards, tom lane

Hi,

On 2019-04-22 20:29:17 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-04-22 16:50:25 -0700, Mikhail Bautin wrote:
> >> What is the standard memory leak checking policy for the PostgreSQL
> >> codebase? I know there is some support for valgrind -- is the test suite
> >> being run continuously with valgrind on the build farm?
> 
> > Leaks are allowed if they are once-per-backend type things. There's no
> > point in e.g. freeing information for timezone metadata, given that
> > it'll be used for the whole server lifetime. And there's such things in
> > psql too, IIRC.
> 
> I would not call the timezone data a "leak", since it's still useful, and
> accessible from static pointers, right up to exit.  A true leak for this
> purpose is memory that's allocated but not usefully accessible, and I'd
> say we discourage that; though small one-time leaks may not be worth the
> trouble to get rid of.

Right. I was only referring to it that way because the various leak
checking tools do, should've been more careful in wording...

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2019-04-22 20:29:17 -0400, Tom Lane wrote:
>> I would not call the timezone data a "leak", since it's still useful, and
>> accessible from static pointers, right up to exit.  A true leak for this
>> purpose is memory that's allocated but not usefully accessible, and I'd
>> say we discourage that; though small one-time leaks may not be worth the
>> trouble to get rid of.

> Right. I was only referring to it that way because the various leak
> checking tools do, should've been more careful in wording...

FWIW, I just did a simple test with valgrind's --leak-check=full,
and I can't find any clear evidence of *any* real leaks in a normal
backend run.  The things that valgrind thinks are leaks seem to be
mostly that it doesn't understand what we're doing.  For example,
(1) it seems to be fooled by pass-by-reference Datums, probably
because the underlying type declaration is an integer type not void*.
(2) it doesn't seem to understand how we manage the element arrays
for dynahash tables, because it claims they're all possibly leaked;
(3) it claims strings passed to putenv() have been leaked;
... etc etc.

Admittedly, this is with RHEL6's valgrind which isn't too modern,
but the net result doesn't really motivate me to spend more time here.

            regards, tom lane