Re: reclaiming memory before exit

[Bruno Haible]

Jeffrey,

> > We discussed this already in the thread starting at [1].
> 
> Not everyone has GNU's low standards.

You are one of the few persons who apply analysis and checking tools to
many GNU projects, more than anyone else [2][3]. This is appreciated (as long
as the reports are about platforms that we care about).

However, your point of view as a "QA engineer" is not the only one that
matters. The thread [1] I pointed to contains some other points of view;
in particular it explains why the GNU standards say something that you find
"low".

Also this thread hinted that the tools could be improved. Consider how many
more projects would use the tools, and how less resistance you would encounter
in your quest for quality, if the tools were better than they are now...

Bruno

[1] https://lists.gnu.org/archive/html/bug-gnulib/2018-10/msg00026.html
[2] https://lists.gnu.org/archive/html/bug-libunistring/2019-05/msg3.html
[3] https://lists.gnu.org/archive/html/bug-gettext/2020-01/msg00028.html

Re: gcc -fanalyze

[Kamil Dudka]

On Friday, May 15, 2020 5:11:40 AM CEST Paul Eggert wrote:
> On 5/14/20 1:34 AM, Kamil Dudka wrote:
> > Now I see that we can just
> > build coreutils with -Dlint even for binary RPMs without any drawbacks.
> 
> Good, and that also means you don't need the coreutils-8.32-if-lint.patch
> that you mentioned earlier. One less thing to maintain.

Thanks for confirmation!  Pushed as:

https://src.fedoraproject.org/rpms/coreutils/c/1f6e0df2

Kamil

Re: reclaiming memory before exit

[Kamil Dudka]

On Friday, May 15, 2020 5:16:28 AM CEST Paul Eggert wrote:
> On 5/14/20 5:11 PM, Jeffrey Walton wrote:
> > Not everyone has GNU's low standards.
> 
> Now may be a good time to remind us of the GNU Kind Communications
> Guideline; see .
> Let's not let honest technical disagreements degenerate into name-calling.

I do not understand why you take it personally.  There are certainly many 
modern software projects that have higher requirements on cleanup of resources 
than what the mentioned text in GNU standards [1] recommends.

How old is the text in GNU standards that Bruno refers to?

There has been some progress mode in software engineering in the recent 
history and the mentioned text might need some update to reflect that.

Kamil

[1] https://lists.gnu.org/archive/html/bug-gnulib/2018-10/msg00029.html

Re: reclaiming memory before exit

[Bruno Haible]

Kamil Dudka wrote:
> How old is the text in GNU standards that Bruno refers to?

How old it is, is irrelevant because the two main points are valid and will
continue to be valid (if the tools don't change):

  1) The sentence "if memory is used until just before a process exits,
 don’t free it simply to silence such a tool" [1] is valid as long
 as GNU targets not an embedded OS for a hardware without MMU, but
 an operation system that frees all resources of a process when the
 process exits.

 A call to 'free (ptr);' right before exit() is a big waste of
 computation resources, because
   - it fetches the memory page of 'ptr' from swap into main memory, or
   - it pulls the memory contents of 'ptr' in the L2 or L1 cache.

 This will not change. And by your argument in this thread, you want
 the program that we run with 'valgrind' (or gcc's or clang's
 LeakSanitizer) during "make check" to be identical to the program
 that you distribute as binaries.

  2) You are going for a goal that you can never fully achieve [2].

You can certainly bring up the issue on the bug-standards list. But you
will have no success, as long as these two points are eliminated.

What can be done to remove these points?

In [2] I suggested to standardize the suppressions file format.

Another possibility is to standardize on a GCC attribute for functions,
with the meaning "memory allocations done inside this functions are
on the same level as static initializations and don't need to be
reclaimed before program exit". valgrind and the leak sanitizers -
which do stack traces anyway - would need to test whether some function
that occur in such stack trace is marked with this GCC attribute.

This could eliminate the problem 2).

For problem 1), maybe trigger the explicit free before exit only if
some environment variable is set?

Bruno

[1] https://www.gnu.org/prep/standards/html_node/Memory-Usage.html
[2] https://lists.gnu.org/archive/html/bug-gnulib/2018-10/msg00037.html

Re: reclaiming memory before exit

[Jeffrey Walton]

On Fri, May 15, 2020 at 3:58 PM Bruno Haible  wrote:
>
> Kamil Dudka wrote:
> > How old is the text in GNU standards that Bruno refers to?
>
> How old it is, is irrelevant because the two main points are valid and will
> continue to be valid (if the tools don't change):
>
>   1) The sentence "if memory is used until just before a process exits,
>  don’t free it simply to silence such a tool" [1] is valid as long
>  as GNU targets not an embedded OS for a hardware without MMU, but
>  an operation system that frees all resources of a process when the
>  process exits.

The use case is incomplete. It does not cover libraries.

Just ask the OpenJDK guys about libraries like Ncurses and OpenSSL.
They were discussing library problems on the OpenSSL mailing list in
the past.

You see, the application will remain loaded, but Java (and other
languages like .Net) will load the shared objects multiple times as
functions are needed (and then unload the shared object when the call
is done).

The subsequent accumulated memory leak eventually crashes the app or device.

Jeff

Re: reclaiming memory before exit

[Paul Eggert]

On 5/15/20 10:52 AM, Kamil Dudka wrote:
> There are certainly many 
> modern software projects that have higher requirements on cleanup of resources

Sure, just as there used to be software projects that had "higher" requirements
on using the Hungarian notation. Although those requirements may have been
justified by the inadequacies of software tools at the time, by today's
standards they were just make-work, and improving the inadequate tools was a
better solution.

For GNU apps, requiring 'free' before program exit is a similar sort of
make-work requirement - it's an artifact of inadequate tools and there's a good
argument that it would be better use of our collective time to fix the tools
rather than to complicate nearly every GNU app to work around the tools'
inadequacies. Bruno made some good suggestions in that direction.

> I do not understand why you take it personally.

I didn't take it personally. The email in question wasn't even directed at me. I
do value civility, though.

Re: reclaiming memory before exit

[Jeffrey Walton]

On Fri, May 15, 2020 at 4:23 PM Paul Eggert  wrote:
>
> On 5/15/20 10:52 AM, Kamil Dudka wrote:
> ...
> > I do not understand why you take it personally.
>
> I didn't take it personally. The email in question wasn't even directed at 
> me. I
> do value civility, though.

There was nothing uncivil or personal about it.

It was a statement of fact. Other GNU projects, like Git and Wget,
recognize the poor advice and disregard it; and get things right on
their own accord. As such, Git and Wget has higher standards then GNU
itself and many of GNU's sister projects. Or, GNU has lower standards
then many other contemporary projects.

To fix this problem you need to fix GNU itself. They need new blood in
leadership that understands and embraces modern engineering processes.
The 1980's advice has outlived its usefulness.

Jeff

Re: c-stack.c and DEBUG: missing import

[Paul Eggert]

On 5/14/20 7:01 AM, Marc Nieper-Wißkirchen wrote:
> This will be a nice use case for the newly created attribute module
> and MAYBE_UNUSED.

I don't think we need to go that far, since c-stack is already using
ignore_value. I installed the attached.

>From c8c5f321dd10b21c8ad55f5c134c93235ffc9930 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 15 May 2020 13:58:30 -0700
Subject: [PATCH] c-stack: pacify -Wunused-result when DEBUG
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Marc Nieper-Wißkirchen in:
https://lists.gnu.org/r/bug-gnulib/2020-05/msg00132.html
* lib/c-stack.c (segv_handler, overflow_handler, segv_handler) [DEBUG]:
Explicitly ignore write failures.
---
 ChangeLog | 8 
 lib/c-stack.c | 6 +++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a3b412818..67af0d448 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2020-05-15  Paul Eggert  
+
+	c-stack: pacify -Wunused-result when DEBUG
+	Problem reported by Marc Nieper-Wißkirchen in:
+	https://lists.gnu.org/r/bug-gnulib/2020-05/msg00132.html
+	* lib/c-stack.c (segv_handler, overflow_handler, segv_handler) [DEBUG]:
+	Explicitly ignore write failures.
+
 2020-05-13  Jim Meyering  
 
 	announce-gen: improve a comment
diff --git a/lib/c-stack.c b/lib/c-stack.c
index e48659130..59606299b 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -172,7 +172,7 @@ segv_handler (void *address _GL_UNUSED, int serious)
 char buf[1024];
 int saved_errno = errno;
 sprintf (buf, "segv_handler serious=%d\n", serious);
-write (STDERR_FILENO, buf, strlen (buf));
+ignore_value (write (STDERR_FILENO, buf, strlen (buf)));
 errno = saved_errno;
   }
 # endif
@@ -195,7 +195,7 @@ overflow_handler (int emergency, stackoverflow_context_t context _GL_UNUSED)
 char buf[1024];
 sprintf (buf, "overflow_handler emergency=%d segv_handler_missing=%d\n",
  emergency, segv_handler_missing);
-write (STDERR_FILENO, buf, strlen (buf));
+ignore_value (write (STDERR_FILENO, buf, strlen (buf)));
   }
 # endif
 
@@ -268,7 +268,7 @@ segv_handler (int signo, siginfo_t *info, void *context _GL_UNUSED)
  "segv_handler fault=%p base=%p size=%lx page=%lx signo=%d\n",
  faulting_address, stack_base, (unsigned long) stack_size,
  (unsigned long) page_size, signo);
-write (STDERR_FILENO, buf, strlen (buf));
+ignore_value (write (STDERR_FILENO, buf, strlen (buf)));
   }
 #   endif
 }
-- 
2.17.1

Re: c-stack.c and DEBUG: missing import

[Bruno Haible]

Hi Paul,

> I don't think we need to go that far, since c-stack is already using
> ignore_value. I installed the attached.

You beat me to it by 2 minutes :)

I verified that the patch fixes the warning that occurred with
  ./configure CPPFLAGS="-Wall -DDEBUG" --with-libsigsegv-prefix=...

Bruno

Re: reclaiming memory at dlclose time

[Bruno Haible]

Jeffrey Walton wrote:
> The use case is incomplete. It does not cover libraries.
> 
> Just ask the OpenJDK guys about libraries like Ncurses and OpenSSL.
> They were discussing library problems on the OpenSSL mailing list in
> the past.
> 
> You see, the application will remain loaded, but Java (and other
> languages like .Net) will load the shared objects multiple times as
> functions are needed (and then unload the shared object when the call
> is done).
> 
> The subsequent accumulated memory leak eventually crashes the app or device.

What you describe here, is a slightly different topic: reclaiming memory
from shared libraries (not programs), at dlclose time (not before exit).

Yes, I confirm that libncurses (libterminfo) has data that it allocates,
like a static initializer would do.

Yes, if a program repeatedly dlopen()s, uses, and dlclose()s such a library,
it will exhibit a memory leak. (Side question: why would anyone dlclose
libncurses or libssl? Sounds wasteful.)

Glibc itself has a number of resource-freeing hooks [1].

GCC allows programmers to mark functions with __attribute__((__destructor__))
[2]. Does glibc walk through these destructors during dlclose()?

Bruno

[1] 
https://sourceware.org/git/?p=glibc.git;a=blob;f=include/libc-symbols.h;hb=HEAD#l245
[2] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

Re: reclaiming memory before exit

[Paul Smith]

On Fri, 2020-05-15 at 16:38 -0400, Jeffrey Walton wrote:
> On Fri, May 15, 2020 at 4:23 PM Paul Eggert 
> wrote:
> > On 5/15/20 10:52 AM, Kamil Dudka wrote:
> > > I do not understand why you take it personally.
> > 
> > I didn't take it personally. The email in question wasn't even
> > directed at me. I do value civility, though.
> 
> It was a statement of fact.

It's an opinion.

You have provided examples of places where final free is important and
necessary (code which is loaded as a shared library and unloaded again,
while the process continues to run).

I can provide examples of places where final free is clearly the wrong
thing: I have worked on programs which run on large systems and can use
100G or 200G or more of RAM: going through all that memory to free it
up, with associated swap reload etc., can cause process shutdown to
take, literally, minutes and cause significant load on the system. 
Simply invoking _exit(), on the other hand, takes a fraction of that
time with little extra load.  In a highly-available system that
difference is absolutely crucial.

Final free is one legitimate way to manage memory at process exit,
certainly, but _exit() (or the equivalent) is also a legitimate choice.
Which one is correct depends on the judgement of the developers, based
on the needs and usage of a particular project, and shouldn't be
subject to dogma on either side.

> To fix this problem you need to fix GNU itself. They need new blood
> in leadership that understands and embraces modern engineering
> processes.

Whatever the intent of the previous comment, this one certainly
violates the Kind Communications guidelines.

Re: reclaiming memory before exit

[Paul Smith]

On Fri, 2020-05-15 at 21:58 +0200, Bruno Haible wrote:
> Another possibility is to standardize on a GCC attribute for
> functions, with the meaning "memory allocations done inside this
> functions are on the same level as static initializations and don't
> need to be reclaimed before program exit".

I think it would be as specific as possible.

One issue is whether it's OK for test suites to leak memory (in the
classic sense) because it's "just a test".  I'm of the opinion that
tests should not contain those types of memory leaks and if someone
comes along with a fix, it should be applied.  I don't think it's
necessary for developers to proactively go looking for these though.

The next set of issues are the ones discussed in the 2018 thread:
singleton objects such as memory kept by bindtextdomain(), init files,
caches, etc.

I don't understand why these are a problem: this memory is not lost. 
Those libraries continue to refer to that memory (perhaps through some
global variable) and as such, memory checkers like valgrind won't
complain about them (at least, that's been my experience).

So what exactly are we talking about here?

There might be memory that is referenced by a local variable in main(),
which would be considered lost if main() returns without freeing it. 
It seems easy enough to turn that into a static variable, for example,
to avoid that check without any downsides or complications in the code.

Alternatively main() could invoke exit() or _exit() and since it never
returns, its memory is still referenced (I don't know whether this will
satisfy memory leak detectors or not).

Are we talking about something else?  Or, are the leak detectors not as
sophisticated as I think about referenced/unreferenced memory and I've
just been lucky with them so far?

Maybe we are just talking about the specific text in the GNU guide,
i.e. this paragraph:

> Memory analysis tools such as valgrind can be useful, but don’t
> complicate a program merely to avoid their false alarms. For example,
> if memory is used until just before a process exits, don’t free it
> simply to silence such a tool.

Are people are reading more into it than it says, and interpreting this
as "GNU programs should not free memory before exit"?  That's not how I
read it.  "Don't complicate programs _only_ to silence memory analysis
tools" leaves plenty of other reasons to free before exit, including
being a shared library, or even just because it's the policy of the
project to do so.  Those seem perfectly fine reasons to me.

However, maybe this language could be made more nuanced.

Re: reclaiming memory before exit

[Bruno Haible]

Hi Paul,

> One issue is whether it's OK for test suites to leak memory (in the
> classic sense) because it's "just a test".  I'm of the opinion that
> tests should not contain those types of memory leaks and if someone
> comes along with a fix, it should be applied.  I don't think it's
> necessary for developers to proactively go looking for these though.

Yes, that's the habit we have adopted in gnulib after some debate. [1]

> The next set of issues are the ones discussed in the 2018 thread:
> singleton objects such as memory kept by bindtextdomain(), init files,
> caches, etc.
> 
> I don't understand why these are a problem: this memory is not lost. 
> Those libraries continue to refer to that memory (perhaps through some
> global variable) and as such, memory checkers like valgrind won't
> complain about them (at least, that's been my experience).

Well, my experience is different. Last month, I analyzed a program
that uses GNU libtextstyle, and the valgrind "leak" reports could be
categorized into three groups:
  * 1056 + 296 + 56 bytes, allocated through cr_style_init_properties
(inside libcroco). This is a statically allocated hash table with
a fixed number of entries. It can be ignored.
  * Allocations inside libcurses. Discussed in this thread: [2].
  * A real leak, which I fixed. [3]

> Or, are the leak detectors not as
> sophisticated as I think about referenced/unreferenced memory and I've
> just been lucky with them so far?

I think it's this, yes.

> Maybe we are just talking about the specific text in the GNU guide,
> i.e. this paragraph:
> 
> > Memory analysis tools such as valgrind can be useful, but don’t
> > complicate a program merely to avoid their false alarms. For example,
> > if memory is used until just before a process exits, don’t free it
> > simply to silence such a tool.
> 
> Are people are reading more into it than it says, and interpreting this
> as "GNU programs should not free memory before exit"?  That's not how I
> read it.  "Don't complicate programs _only_ to silence memory analysis
> tools" leaves plenty of other reasons to free before exit, including
> being a shared library, or even just because it's the policy of the
> project to do so.  Those seem perfectly fine reasons to me.
> 
> However, maybe this language could be made more nuanced.

The discussion is not only about language nuances. There are people
who request that GNU programs should free all memory before exit(),
_only_ to silence memory analysis tools.

Like it was with "gcc -Wall" warnings. 20 years ago, people like me
were saying "let me use 'gcc -Wall' since it produces useful warnings;
some warnings can remain since I know them". But other people insisted
that no warnings should remain, because they want to see new warnings
clearly and immediately, without having to learn beforehand which
warnings they need to expect.

It's similar with the valgrind output. The tool is more useful if,
in the normal situation, it produces no stack traces, than if it normally
shows 25 stack traces, from libncurses, libcroco, and libtextstyle,
all intermixed.

That's what the discussion is about.

Bruno

[1] https://lists.gnu.org/archive/html/bug-gnulib/2019-03/msg00025.html
[2] https://lists.gnu.org/archive/html/bug-ncurses/2009-04/msg00028.html
[3] 
https://git.savannah.gnu.org/gitweb/?p=gettext.git;a=commitdiff;h=1d166f60e79d39d2ba1342e68f96104c24c85fb7

Re: reclaiming memory before exit

[Jeffrey Walton]

On Fri, May 15, 2020 at 6:07 PM Paul Smith  wrote:
>
> On Fri, 2020-05-15 at 16:38 -0400, Jeffrey Walton wrote:
> > On Fri, May 15, 2020 at 4:23 PM Paul Eggert 
>
> > To fix this problem you need to fix GNU itself. They need new blood
> > in leadership that understands and embraces modern engineering
> > processes.
>
> Whatever the intent of the previous comment, this one certainly
> violates the Kind Communications guidelines.

Allow me to rephrase again as if I am speaking to executives: GNU
should embrace a diversity of leadership.

Jeff

Re: Remove license modules

[Bruno Haible]

Hi,

Asher Gordon wrote:
> Since Savannah requires licenses to be under version control (see
> here¹), I don't think it makes sense for Gnulib to provide modules for
> them. My reasoning is that if developers are going to use a Gnulib
> module, they won't keep the files it provides under version control (in
> fact, gnulib-tool itself adds these files to .gitignore). There's no
> reason to remove the license *files*, since it could be useful for
> developers to copy them directly.
> ...
> Footnotes: 
> ¹  https://savannah.nongnu.org/task/?func=detailitem&item_id=15583

At the moment you sent the mail, the discussion in that ticket was still
ongoing.

Now it has completed, and it turned out you were confusing the license
texts in text format (which ought to be added to VCS directly) and the
license texts in Texinfo format (for which gnulib modules are appropriate).

Bruno

Re: reclaiming memory before exit

[Bruno Haible]

Jeffrey Walton wrote:
> Allow me to rephrase again as if I am speaking to executives: GNU
> should embrace a diversity of leadership.

This topic is off-topic on this list. Please don't discuss it here.
First, read [1]. Then, you can discuss it privately with Richard
Stallman and the members of the GNU Advisory Comittee [2].

Bruno

[1] https://www.gnu.org/gnu/gnu-structure.en.html
[2] https://www.gnu.org/contact/gnu-advisory.en.html

Re: reclaiming memory before exit

[Paul Smith]

On Sat, 2020-05-16 at 01:03 +0200, Bruno Haible wrote:
> Well, my experience is different. Last month, I analyzed a program
> that uses GNU libtextstyle, and the valgrind "leak" reports could be
> categorized into three groups:
>   * 1056 + 296 + 56 bytes, allocated through cr_style_init_properties
> (inside libcroco). This is a statically allocated hash table with
> a fixed number of entries. It can be ignored.
>   * Allocations inside libcurses. Discussed in this thread: [2].
>   * A real leak, which I fixed. [3]

Interesting.  I have run valgrind many times on GNU make, which uses a
lot of allocated memory which is referred to by global variables and is
not freed at exit, and I don't recall seeing these reported as leaks.

I will have to run again and refresh my memory as it's been some months
since I tried it last.

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.