Run thread shutdown functions in an explicit order

This removes the __libc_thread_subfreeres hook in favor of explict
calls.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

diff --git a/ChangeLog b/ChangeLog
index 5cd85668d5ec57762d0cae2afac6de232f5de6eb..eaee727677b9e053b23f78af5f5f0b270d294f60 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,33 @@
+2018-06-26  Florian Weimer  <fweimer@redhat.com>
+
+       Run thread shutdown functions in an explicit order.
+       * malloc/thread-freeres.c (__libc_thread_subfreeres): Remove hook
+       definition.
+       (__libc_thread_freeres): Call thread shutdown functions
+       explicitly.
+       * include/rpc/rpc.h (__rpc_thread_destroy): Add hidden attribute.
+       * include/string.h (__strerror_thread_freeres): Declare.
+       * malloc/arena.c (__malloc_arena_thread_freeres): Renamed from
+       arena_thread_freeres.  No longer static.  Remove thread shutdown
+       hook registration.
+       * malloc/malloc-internal.h (__malloc_arena_thread_freeres):
+       Declare.
+       * resolv/res-close.c (__res_thread_freeres): Renamed from
+       res_thread_freeres.  No longer static.  Remove thread shutdown
+       hook registration.
+       * resolv/resolv-internal.h (__res_thread_freeres): Declare.
+       * resolv/resolv_conf.c (freeres): Remove incorrect section
+       attribute and use libc_freeres_fn.
+       * string/strerror_l.c (__strerror_thread_freeres): Renamed from
+       strerror_thread_freeres.  No longer static.  Remove thread
+       shutdown hook registration.
+       * sysdeps/mach/strerror_l.c (__strerror_thread_freeres): Likewise.
+       * sunrpc/rpc_thread.c (__rpc_thread_destroy): Remove thread
+       shutdown hook registration.
+       * Makerules (shlib.lds): Do not provide section boundary symbols
+       for __libc_thread_subfreeres.
+       * manual/memory.texi (Basic Allocation): Update comment.
+
 2018-06-26  Florian Weimer  <fweimer@redhat.com>
 
        Remove always-defined _RPC_THREAD_SAFE_ macro.

diff --git a/Makerules b/Makerules
index b2c2724fcbd4a7e8e9462c34e375f0b3736f8670..a10a0b4d7021e8ecb517ab57bef5e630892fd814 100644 (file)
--- a/Makerules
+++ b/Makerules
@@ -648,9 +648,6 @@ $(common-objpfx)shlib.lds: $(common-objpfx)config.make $(..)Makerules
                 PROVIDE(__start___libc_atexit = .);\
                 __libc_atexit : { *(__libc_atexit) }\
                 PROVIDE(__stop___libc_atexit = .);\
-                PROVIDE(__start___libc_thread_subfreeres = .);\
-                __libc_thread_subfreeres : { *(__libc_thread_subfreeres) }\
-                PROVIDE(__stop___libc_thread_subfreeres = .);\
                 PROVIDE(__start___libc_IO_vtables = .);\
                 __libc_IO_vtables : { *(__libc_IO_vtables) }\
                 PROVIDE(__stop___libc_IO_vtables = .);\

diff --git a/include/rpc/rpc.h b/include/rpc/rpc.h
index 327d84319e8b3278bf1e2eaa0afedaf088eb12ae..f5cee6caef6284d25fd8adac047cde31749eb11a 100644 (file)
--- a/include/rpc/rpc.h
+++ b/include/rpc/rpc.h
@@ -45,7 +45,7 @@ extern void __rpc_thread_svc_cleanup (void) attribute_hidden;
 extern void __rpc_thread_clnt_cleanup (void) attribute_hidden;
 extern void __rpc_thread_key_cleanup (void) attribute_hidden;
 
-extern void __rpc_thread_destroy (void);
+extern void __rpc_thread_destroy (void) attribute_hidden;
 
 __libc_tsd_define (extern, struct rpc_thread_variables *, RPC_VARS)
 

diff --git a/include/string.h b/include/string.h
index bb4922cbbe65dd438059106d2ce69508d201e16e..4d622f1c0305e78ebbf18a71adb27caff33c0bcd 100644 (file)
--- a/include/string.h
+++ b/include/string.h
@@ -50,6 +50,9 @@ extern int __ffs (int __i) __attribute__ ((const));
 
 extern char *__strerror_r (int __errnum, char *__buf, size_t __buflen);
 
+/* Called as part of the thread shutdown sequence.  */
+void __strerror_thread_freeres (void) attribute_hidden;
+
 /* Get _STRING_ARCH_unaligned.  */
 #include <string_private.h>
 #endif

diff --git a/malloc/arena.c b/malloc/arena.c
index 37183cfb6ab5d0735cc82759626670aff3832cd0..497ae475e7a859025ef8a2e1c42ec934a4700dbd 100644 (file)
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -941,8 +941,8 @@ arena_get_retry (mstate ar_ptr, size_t bytes)
   return ar_ptr;
 }
 
-static void __attribute__ ((section ("__libc_thread_freeres_fn")))
-arena_thread_freeres (void)
+void
+__malloc_arena_thread_freeres (void)
 {
   /* Shut down the thread cache first.  This could deallocate data for
      the thread arena, so do this before we put the arena on the free
@@ -966,7 +966,6 @@ arena_thread_freeres (void)
       __libc_lock_unlock (free_list_lock);
     }
 }
-text_set_element (__libc_thread_subfreeres, arena_thread_freeres);
 
 /*
  * Local variables:

diff --git a/malloc/malloc-internal.h b/malloc/malloc-internal.h
index ad054508ee44162d09510b945dff6451351fae44..9cee0fb2d7e0994b065869a570c1d6526c5a8bbc 100644 (file)
--- a/malloc/malloc-internal.h
+++ b/malloc/malloc-internal.h
@@ -71,6 +71,9 @@ void __malloc_fork_unlock_parent (void) attribute_hidden;
 /* Called in the child process after a fork.  */
 void __malloc_fork_unlock_child (void) attribute_hidden;
 
+/* Called as part of the thread shutdown sequence.  */
+void __malloc_arena_thread_freeres (void) attribute_hidden;
+
 /* Set *RESULT to LEFT * RIGHT.  Return true if the multiplication
    overflowed.  */
 static inline bool

diff --git a/malloc/thread-freeres.c b/malloc/thread-freeres.c
index 53ce41bb785113784b65e0ce0e04f17eb88d5489..8902c845bcfdb6d5e2f85764edfdc2700cc3503b 100644 (file)
--- a/malloc/thread-freeres.c
+++ b/malloc/thread-freeres.c
@@ -16,16 +16,24 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <stdlib.h>
 #include <libc-internal.h>
-#include <set-hooks.h>
+#include <malloc-internal.h>
+#include <resolv/resolv-internal.h>
+#include <rpc/rpc.h>
+#include <string.h>
 
-#ifdef _LIBC_REENTRANT
-DEFINE_HOOK (__libc_thread_subfreeres, (void));
-
-void __attribute__ ((section ("__libc_thread_freeres_fn")))
+/* Thread shutdown function.  Note that this function must be called
+   for threads during shutdown for correctness reasons.  Unlike
+   __libc_subfreeres, skipping calls to it is not a valid
+   optimization.  */
+void
 __libc_thread_freeres (void)
 {
-  RUN_HOOK (__libc_thread_subfreeres, ());
+  call_function_static_weak (__rpc_thread_destroy);
+  call_function_static_weak (__res_thread_freeres);
+  call_function_static_weak (__strerror_thread_freeres);
+
+  /* This should come last because it shuts down malloc for this
+     thread and the other shutdown functions might well call free.  */
+  call_function_static_weak (__malloc_arena_thread_freeres);
 }
-#endif

diff --git a/manual/memory.texi b/manual/memory.texi
index b95f6aa1b9b8fe658fccfb92165f7084808760c5..2fac64939fa77486aa7465684663054422a07c28 100644 (file)
--- a/manual/memory.texi
+++ b/manual/memory.texi
@@ -404,7 +404,7 @@ this function is in @file{stdlib.h}.
 @c      reads&writes next_to_use and iterates over arena next without guards
 @c      those are harmless as long as we don't drop arenas from the
 @c      NEXT list, and we never do; when a thread terminates,
-@c      arena_thread_freeres prepends the arena to the free_list
+@c      __malloc_arena_thread_freeres prepends the arena to the free_list
 @c      NEXT_FREE list, but NEXT is never modified, so it's safe!
 @c     mutex_trylock (arena lock) @asulock @aculock
 @c     mutex_lock (arena lock) dup @asulock @aculock

diff --git a/resolv/res-close.c b/resolv/res-close.c
index e02f5afa1597aee09eaa971a8b8f889726c436d4..38572b1d2f7ea502b6608cc1bd8b0a5b009f69fa 100644 (file)
--- a/resolv/res-close.c
+++ b/resolv/res-close.c
@@ -126,8 +126,8 @@ res_nclose (res_state statp)
 libc_hidden_def (__res_nclose)
 
 /* This is called when a thread is exiting to free resources held in _res.  */
-static void __attribute__ ((section ("__libc_thread_freeres_fn")))
-res_thread_freeres (void)
+void
+__res_thread_freeres (void)
 {
   __resolv_context_freeres ();
 
@@ -140,5 +140,4 @@ res_thread_freeres (void)
   /* Make sure we do a full re-initialization the next time.  */
   _res.options = 0;
 }
-text_set_element (__libc_thread_subfreeres, res_thread_freeres);
-text_set_element (__libc_subfreeres, res_thread_freeres);
+text_set_element (__libc_subfreeres, __res_thread_freeres);

diff --git a/resolv/resolv-internal.h b/resolv/resolv-internal.h
index ac6e495927bc4b782f842970553f392a12831786..b8e447c726c8f434a9c82f4e6ab8da1c07916e85 100644 (file)
--- a/resolv/resolv-internal.h
+++ b/resolv/resolv-internal.h
@@ -97,4 +97,7 @@ int __res_nopt (struct resolv_context *, int n0,
 int __inet_pton_length (int af, const char *src, size_t srclen, void *);
 libc_hidden_proto (__inet_pton_length)
 
+/* Called as part of the thread shutdown sequence.  */
+void __res_thread_freeres (void) attribute_hidden;
+
 #endif  /* _RESOLV_INTERNAL_H */

diff --git a/resolv/resolv_conf.c b/resolv/resolv_conf.c
index ab4e5dc82caafcb31c3dab9f002995b1a4e26a08..b4021ab735d200710d57a5cde881792de537f5aa 100644 (file)
--- a/resolv/resolv_conf.c
+++ b/resolv/resolv_conf.c
@@ -673,8 +673,7 @@ __resolv_conf_detach (struct __res_state *resp)
 }
 
 /* Deallocate the global data.  */
-static void __attribute__ ((section ("__libc_thread_freeres_fn")))
-freeres (void)
+libc_freeres_fn (freeres)
 {
   /* No locking because this function is supposed to be called when
      the process has turned single-threaded.  */
@@ -698,4 +697,3 @@ freeres (void)
      deallocated memory.  */
   global = NULL;
 }
-text_set_element (__libc_subfreeres, freeres);

diff --git a/string/strerror_l.c b/string/strerror_l.c
index 7d90e809ceda161e210fe80e484f3312e68c55ea..2a9c3b5e0b2559d2499b33db4005b093f07c730f 100644 (file)
--- a/string/strerror_l.c
+++ b/string/strerror_l.c
@@ -57,15 +57,9 @@ strerror_l (int errnum, locale_t loc)
 }
 
 
-#ifdef _LIBC
-# ifdef _LIBC_REENTRANT
-/* This is called when a thread is exiting to free the last_value string.  */
-static void __attribute__ ((section ("__libc_thread_freeres_fn")))
-strerror_thread_freeres (void)
+void
+__strerror_thread_freeres (void)
 {
   free (last_value);
 }
-text_set_element (__libc_thread_subfreeres, strerror_thread_freeres);
-text_set_element (__libc_subfreeres, strerror_thread_freeres);
-# endif
-#endif
+text_set_element (__libc_subfreeres, __strerror_thread_freeres);

diff --git a/sunrpc/rpc_thread.c b/sunrpc/rpc_thread.c
index 068a49f92b2ff79ab172600e826ab4f2802d130d..a65a90dc15f278edde53b3352db0a2fd201c3edc 100644 (file)
--- a/sunrpc/rpc_thread.c
+++ b/sunrpc/rpc_thread.c
@@ -15,7 +15,7 @@ static __thread struct rpc_thread_variables *thread_rpc_vars
 /*
  * Task-variable destructor
  */
-void __attribute__ ((section ("__libc_thread_freeres_fn")))
+void
 __rpc_thread_destroy (void)
 {
        struct rpc_thread_variables *tvp = thread_rpc_vars;
@@ -36,12 +36,8 @@ __rpc_thread_destroy (void)
                thread_rpc_vars = NULL;
        }
 }
-#ifdef _LIBC_REENTRANT
-text_set_element (__libc_thread_subfreeres, __rpc_thread_destroy);
-#endif
 text_set_element (__libc_subfreeres, __rpc_thread_destroy);
 
-
 /*
  * Initialize RPC multi-threaded operation
  */

diff --git a/sysdeps/mach/strerror_l.c b/sysdeps/mach/strerror_l.c
index 61515a46b8bf01fe60dabc68fea99fd98e2d8714..b9842ccf40b71afbe41f1eeb37cff8004b50fed9 100644 (file)
--- a/sysdeps/mach/strerror_l.c
+++ b/sysdeps/mach/strerror_l.c
@@ -87,15 +87,9 @@ strerror_l (int errnum, locale_t loc)
 }
 
 
-#ifdef _LIBC
-# ifdef _LIBC_REENTRANT
-/* This is called when a thread is exiting to free the last_value string.  */
-static void __attribute__ ((section ("__libc_thread_freeres_fn")))
-strerror_thread_freeres (void)
+void
+__strerror_thread_freeres (void)
 {
   free (last_value);
 }
-text_set_element (__libc_thread_subfreeres, strerror_thread_freeres);
-text_set_element (__libc_subfreeres, strerror_thread_freeres);
-# endif
-#endif
+text_set_element (__libc_subfreeres, __strerror_thread_freeres);