reclaiming memory before exit, take 2

[Bruno Haible]

Hi all,

Let me try to bring some structure into this discussion.

1) The memory-leak tools
2) The developer's perspective
3) The QA automation perspective


1) The memory-leak tools
========================

Paul and I apparently use the tools differently [1]. So, I've written
a wiki page
  https://gitlab.com/ghwiki/gnow-how/-/wikis/Finding_memory_leaks
that describes what memory leaks are, what tools exist to find them,
what are advantages and drawbacks. Please read it.

(If you want to contribute to this wiki, drop me a note, and I'll give
you write access.)

In particular, all three tools by default omit memory leaks caused by
global and static variables.

Paul writes in [1]:
> In any event, it seems to me to be a deficiency in the detection if it
> reports allocated memory which is still referenced to by global
> variables, or even static variables, as memory leaks.

On the contrary, reporting leaks through global and static variables is
a feature. *Hiding* them, which is what the tools do by default, is a
*misfeature*.

Why? Because caches that grow without bounds are serious memory leaks
as well. It is pointless to eliminate serious memory leaks that were
caused by forgetting free(), while at the same not noticing serious memory
leaks that were caused by bad design of caches. BOTH have same adverse
effects.

This means, when looking for memory leaks, you should use valgrind with
'--show-reachable=yes'.


2) The developer's perspective
==============================

A developer will want to eliminate all serious memory leaks and not spend
much time on the not-serious ones. However:

  The tools cannot distinguish serious from not serious memory leaks.

Therefore the developer will typically want to silence the not serious
memory leaks (like they also want to silence "gcc -Wall" warnings when
they are pointless).

I can see three good and one bad ways of doing so:

* [good] Enhance the suppressions file(s).

* [good] In unit tests, add free() statements right before exit(). Like
  Paul said in [2]:
    "tests should not contain those types of memory leaks and if someone
     comes along with a fix, it should be applied."

* [acceptable] In production binaries, add free() statements guarded by an
  environment variable test:

    if (getenv ("CLEANUP_BEFORE_EXIT") != NULL)
      free (data);
    exit (0);

  Removing the getenv test would, however, violate the GNU standards [3].

* [bad] Omit the valgrind option '--show-reachable=yes'.
  This is bad because, as explained above, it will make you blind against
  some types of serious memory leaks.


3) The QA automation perspective
================================

For QA automation, a multitude of program executions are done with a
memory leak checker enabled. Since it needs to be automated, the usual
policy will be that a valgrind or leak sanitizer report is considered
a failure.

The QA automation needs to rely on the suppression files created by the
developers, since it's not a QA engineer's job to evaluate whether a
particular memory leak is serious or not.

A good QA automation will thus use 'valgrind --show-reachable=yes'
and the developer's suppression files.


Bruno

[1] https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00154.html
[2] https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00149.html
[3] https://www.gnu.org/prep/standards/html_node/Memory-Usage.html