[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: fstrcmp: memory is not reclaimed on exit

From: Bruno Haible
Subject: Re: fstrcmp: memory is not reclaimed on exit
Date: Sun, 16 Feb 2020 13:52:48 +0100
User-agent: KMail/5.1.3 (Linux/4.4.0-171-generic; KDE/5.18.0; x86_64; ; )

Hi Akim,

Sorry for the delay.

> > Do the threads still exist at the moment valgrind does its inventory of 
> > left-
> > over memory?
> I don't know when it runs its stuff, but I expect, given where it stands,
> that it does it as the latest possible instant.
> > In particular:
> >  - Did you create threads, in which fstrcmp is run? If yes, are they still
> >    running?
> No, Bison does not use any threads.

OK, then valgrind is really complaining about the memory allocations done in the
main thread.

> >  - Or did you run fstrcmp in the main thread? Most likely valgrind does its
> >    inventory in the main thread, during exit(). This means that at this 
> > point
> >    the fstrcmp buffer for the main thread still exists. In other words, you
> >    should treat thread-local memory allocations for the main thread like
> >    static memory allocations (e.g. like in uniqstr.c).
> I agree, I would like to be able to explicitly release the memory.  But
> I can't see any API to do that in fstrcmp.c.  Is this one ok?

The GNU Coding Standards [1] tell us to not worry about this situation. However,
the alternative - to check for a memory leak - would be to run the test in a
loop (e.g. 1000 times), which is not practical since bison tests take some
noticeable time to execute already when executed once. Therefore, I'm OK to
look at a workaround.

> +void
> +fstrcmp_free (void)
> +{
> +  gl_once (keys_init_once, keys_init);
> +  gl_tls_key_destroy (buffer_key);
> +  gl_tls_key_destroy (bufmax_key);
> +}

This workaround is insufficient, since POSIX [2] says that
   "It is the responsibility of the application to free any application
    storage or perform any cleanup actions for data structures related
    to the deleted key or associated thread-specific data in any threads"

In other words, pthread_key_delete is not guaranteed to call the destructor
of 'buffer_key'. The gnulib test (tests/test-tls.c functions test_tls_dtorcheck1
and test_tls_dtorcheck2) verifies that the destructor gets called, but only
for threads != main thread, and you are interested in the main thread
particularly. Most likely, in this test, the destructor gets called when the
thread exits [3], not when pthread_key_delete gets called.

[1] https://www.gnu.org/prep/standards/html_node/Memory-Usage.html

This patch, however, should work.

2020-02-16  Bruno Haible  <address@hidden>

        fstrcmp: Add API to clean up resources.
        Reported by Akim Demaille <address@hidden> in
        * lib/fstrcmp.h (fstrcmp_free_resources): New declaration.
        * lib/fstrcmp.c (fstrcmp_free_resources): New function.

diff --git a/lib/fstrcmp.c b/lib/fstrcmp.c
index c6a6638..1a4fbfd 100644
--- a/lib/fstrcmp.c
+++ b/lib/fstrcmp.c
@@ -73,6 +73,21 @@ keys_init (void)
 /* Ensure that keys_init is called once only.  */
 gl_once_define(static, keys_init_once)
+fstrcmp_free_resources (void)
+  ptrdiff_t *buffer;
+  gl_once (keys_init_once, keys_init);
+  buffer = gl_tls_get (buffer_key);
+  if (buffer != NULL)
+    {
+      gl_tls_set (buffer_key, NULL);
+      gl_tls_set (bufmax_key, (void *) (uintptr_t) 0);
+      free (buffer);
+    }
 /* In the code below, branch probabilities were measured by Ralf Wildenhues,
    by running "msgmerge LL.po coreutils.pot" with msgmerge 0.18 for many
diff --git a/lib/fstrcmp.h b/lib/fstrcmp.h
index 92b67e3..37df588 100644
--- a/lib/fstrcmp.h
+++ b/lib/fstrcmp.h
@@ -38,6 +38,15 @@ extern double fstrcmp_bounded (const char *s1, const char 
 /* A shortcut for fstrcmp.  Avoids a function call.  */
 #define fstrcmp(s1,s2) fstrcmp_bounded (s1, s2, 0.0)
+/* Frees the per-thread resources allocated by this module for the current
+   thread.
+   You don't need to call this function in threads other than the main thread,
+   because per-thread resources are reclaimed automatically when the thread
+   exits.  However, per-thread resources allocated by the main thread are
+   comparable to static allocations; calling this function can be useful to
+   avoid an error report from valgrind.  */
+extern void fstrcmp_free_resources (void);
 #ifdef __cplusplus

reply via email to

[Prev in Thread] Current Thread [Next in Thread]