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
[2]
https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_key_delete.html
[3]
https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_key_create.html
This patch, however, should work.
2020-02-16 Bruno Haible <br...@clisp.org>
fstrcmp: Add API to clean up resources.
Reported by Akim Demaille <a...@lrde.epita.fr> in
<https://lists.gnu.org/archive/html/bug-gnulib/2020-01/msg00080.html>.
* 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)
+void
+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
*s2,
/* 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
}
#endif
