Re: fstrcmp: memory is not reclaimed on exit

[Akim Demaille]

Hi Bruno,

> Le 20 janv. 2020 à 22:57, Bruno Haible <address@hidden> a écrit :
> 
>>>     Initialization:            gl_tls_key_init (name, destructor);
>>> 
>>>   A destructor is a function pointer of type 'void (*) (void *)', called
>>>   when a thread exits, and taking the last per-thread value as argument.  It
>>>   is unspecified whether the destructor function is called when the last
>>>   per-thread value is NULL.  On some platforms, the destructor function is
>>>   not called at all.
>> 
>> I can see that it's not expected to work on some platforms, but in the
>> case of the user the platform is fairly mainstream:
> 
> Meanwhile this destructor stuff even works on native Windows. The list of
> platforms where it does not work is very small (something like mingw with
> winpthreads, IIRC).

Great!

>> So I don't know what to do.  Is this a red herring related to Valgrind
>> as a platform?
> 
> 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.

>  - 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?  I feel
stupid to initialize the memory right before releasing, but I didn't
find a means to check whether the tls memory was initialized.

Thanks!

commit eee1a395a841f7d1ae4388710c88c5dd3e047cc0
Author: Akim Demaille <address@hidden>
Date:   Wed Jan 22 07:46:45 2020 +0100

    fstrcmp: provide a means to explictly release resources
    
    * lib/fstrcmp.h, lib/fstrcmp.c (fstrcmp_free): New.

diff --git a/ChangeLog b/ChangeLog
index 53a1fb435..10626e535 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-22  Akim Demaille  <address@hidden>
+
+       fstrcmp: provide a means to explictly release resources.
+       * lib/fstrcmp.h, lib/fstrcmp.c (fstrcmp_free): New.
+
 2020-01-09  Bruno Haible  <address@hidden>
 
        c32srtombs: Add tests.
diff --git a/lib/fstrcmp.c b/lib/fstrcmp.c
index c6a66389e..e8e02e856 100644
--- a/lib/fstrcmp.c
+++ b/lib/fstrcmp.c
@@ -73,6 +73,13 @@ keys_init (void)
 /* Ensure that keys_init is called once only.  */
 gl_once_define(static, keys_init_once)
 
+void
+fstrcmp_free (void)
+{
+  gl_once (keys_init_once, keys_init);
+  gl_tls_key_destroy (buffer_key);
+  gl_tls_key_destroy (bufmax_key);
+}
 
 /* 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 92b67e34a..f3a57ecb6 100644
--- a/lib/fstrcmp.h
+++ b/lib/fstrcmp.h
@@ -38,6 +38,13 @@ 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)
 
+/* Explicitly release resources acquired in this thread.  Regular
+   thread termination invokes it automatically, so it is typically not
+   needed to call it.  However, in the case of the main thread, tools
+   such as Valgrind might report "leaks" if the memory is not
+   explicitly reclaimed first.  */
+extern void fstrcmp_free (void);
+
 #ifdef __cplusplus
 }
 #endif