Re: [lttng-dev] [PATCH lttng-ust v3] Fix: destructors do not run on probe provider dlclose

2018-02-26 Thread Mathieu Desnoyers
- On Feb 26, 2018, at 3:15 PM, Francis Deslauriers 
francis.deslauri...@efficios.com wrote:

> Calling dlclose on the probe provider library that first loaded
> __tracepoints__disable_destructors in the symbol table does not
> unregister the probes from the callsites as the destructors are not
> executed.
> 
> The __tracepoints__disable_destructors weak symbol is exposed by probe
> providers, liblttng-ust.so and liblttng-ust-tracepoint.so libraries. If
> a probe provider is loaded first into the address space, its definition
> is bound to the symbol. All the subsequent loaded libraries using the
> symbol will use the existing definition of the symbol, thus creating a
> situation where liblttng-ust.so or liblttng-ust-tracepoint.so depend on
> the probe provider library.
> 
> This prevents the dynamic loader from unloading the library as it is
> still in use by other libraries. Because of this, the execution of its
> destructors and the unregistration of the probes is postponed.
> 
> To overcome this issue, we no longer expose this symbol in the
> tracepoint.h file to remove the explicit dependency of the probe
> provider on the symbol. We instead use the existing dlopen handle on
> liblttng-ust-tracepoint.so to get handles on getter and setter functions
> for this value.
> 
> Signed-off-by: Francis Deslauriers 
> ---
> include/lttng/tracepoint.h | 82 +++---
> liblttng-ust/tracepoint.c  | 61 ++
> 2 files changed, 124 insertions(+), 19 deletions(-)
> 
> diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
> index 39f2c4d..d285347 100644
> --- a/include/lttng/tracepoint.h
> +++ b/include/lttng/tracepoint.h
> @@ -226,21 +226,6 @@ struct lttng_ust_tracepoint_dlopen {
> extern struct lttng_ust_tracepoint_dlopen tracepoint_dlopen;
> extern struct lttng_ust_tracepoint_dlopen *tracepoint_dlopen_ptr;
> 
> -/* Disable tracepoint destructors. */
> -int __tracepoints__disable_destructors __attribute__((weak));
> -
> -/*
> - * Programs that have threads that survive after they exit, and
> - * therefore call library destructors, should disable the tracepoint
> - * destructors by calling tracepoint_disable_destructors(). This will
> - * leak the tracepoint instrumentation library shared object, leaving
> - * its teardown to the operating system process teardown.
> - */
> -static inline void tracepoint_disable_destructors(void)
> -{
> - __tracepoints__disable_destructors = 1;
> -}
> -
> /*
>  * These weak symbols, the constructor, and destructor take care of
>  * registering only _one_ instance of the tracepoints per shared-ojbect
> @@ -265,6 +250,47 @@ struct lttng_ust_tracepoint_dlopen tracepoint_dlopen
> struct lttng_ust_tracepoint_dlopen *tracepoint_dlopen_ptr
>   __attribute__((weak, visibility("hidden")));
> 
> +/*
> + * Tracepoint dynamic linkage handling (callbacks). Hidden visibility: shared
> + * across objects in a module/main executable. The callbacks are used to
> + * control and check if the destructors should be executed.
> + */
> +struct lttng_ust_tracepoint_destructors_syms {
> + void (*tracepoint_enable_destructors)(void);
> + void (*tracepoint_disable_destructors)(void);
> + int (*tracepoint_get_destructors_state)(void);
> +};
> +
> +extern struct lttng_ust_tracepoint_destructors_syms
> tracepoint_destructors_syms;
> +extern struct lttng_ust_tracepoint_destructors_syms
> *tracepoint_destructors_syms_ptr;
> +
> +struct lttng_ust_tracepoint_destructors_syms tracepoint_destructors_syms
> + __attribute__((weak, visibility("hidden")));
> +struct lttng_ust_tracepoint_destructors_syms *tracepoint_destructors_syms_ptr
> + __attribute__((weak, visibility("hidden")));
> +
> +static inline void tracepoint_disable_destructors(void)
> +{
> + if (!tracepoint_dlopen_ptr)
> + tracepoint_dlopen_ptr = &tracepoint_dlopen;
> + if (!tracepoint_destructors_syms_ptr)
> + tracepoint_destructors_syms_ptr = &tracepoint_destructors_syms;
> + if (tracepoint_dlopen_ptr->liblttngust_handle
> + && 
> tracepoint_destructors_syms_ptr->tracepoint_disable_destructors)
> + 
> tracepoint_destructors_syms_ptr->tracepoint_disable_destructors();
> +}
> +
> +static inline void tracepoint_enable_destructors(void)
> +{
> + if (!tracepoint_dlopen_ptr)
> + tracepoint_dlopen_ptr = &tracepoint_dlopen;
> + if (!tracepoint_destructors_syms_ptr)
> + tracepoint_destructors_syms_ptr = &tracepoint_destructors_syms;
> + if (tracepoint_dlopen_ptr->liblttngust_handle
> + && 
> tracepoint_destructors_syms_ptr->tracepoint_enable_destructors)
> + 
> tracepoint_destructors_syms_ptr->tracepoint_enable_destructors();
> +}
> +
> #ifndef _LGPL_SOURCE
> static inline void lttng_ust_notrace
> __tracepoint__init_urcu_sym(void);
> @@ -335,8 +361,11 @@ __tracepoints__destroy(void)
>   return;
>   if (!trac

[lttng-dev] [PATCH lttng-ust v3] Fix: destructors do not run on probe provider dlclose

2018-02-26 Thread Francis Deslauriers
Calling dlclose on the probe provider library that first loaded
__tracepoints__disable_destructors in the symbol table does not
unregister the probes from the callsites as the destructors are not
executed.

The __tracepoints__disable_destructors weak symbol is exposed by probe
providers, liblttng-ust.so and liblttng-ust-tracepoint.so libraries. If
a probe provider is loaded first into the address space, its definition
is bound to the symbol. All the subsequent loaded libraries using the
symbol will use the existing definition of the symbol, thus creating a
situation where liblttng-ust.so or liblttng-ust-tracepoint.so depend on
the probe provider library.

This prevents the dynamic loader from unloading the library as it is
still in use by other libraries. Because of this, the execution of its
destructors and the unregistration of the probes is postponed.

To overcome this issue, we no longer expose this symbol in the
tracepoint.h file to remove the explicit dependency of the probe
provider on the symbol. We instead use the existing dlopen handle on
liblttng-ust-tracepoint.so to get handles on getter and setter functions
for this value.

Signed-off-by: Francis Deslauriers 
---
 include/lttng/tracepoint.h | 82 +++---
 liblttng-ust/tracepoint.c  | 61 ++
 2 files changed, 124 insertions(+), 19 deletions(-)

diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
index 39f2c4d..d285347 100644
--- a/include/lttng/tracepoint.h
+++ b/include/lttng/tracepoint.h
@@ -226,21 +226,6 @@ struct lttng_ust_tracepoint_dlopen {
 extern struct lttng_ust_tracepoint_dlopen tracepoint_dlopen;
 extern struct lttng_ust_tracepoint_dlopen *tracepoint_dlopen_ptr;
 
-/* Disable tracepoint destructors. */
-int __tracepoints__disable_destructors __attribute__((weak));
-
-/*
- * Programs that have threads that survive after they exit, and
- * therefore call library destructors, should disable the tracepoint
- * destructors by calling tracepoint_disable_destructors(). This will
- * leak the tracepoint instrumentation library shared object, leaving
- * its teardown to the operating system process teardown.
- */
-static inline void tracepoint_disable_destructors(void)
-{
-   __tracepoints__disable_destructors = 1;
-}
-
 /*
  * These weak symbols, the constructor, and destructor take care of
  * registering only _one_ instance of the tracepoints per shared-ojbect
@@ -265,6 +250,47 @@ struct lttng_ust_tracepoint_dlopen tracepoint_dlopen
 struct lttng_ust_tracepoint_dlopen *tracepoint_dlopen_ptr
__attribute__((weak, visibility("hidden")));
 
+/*
+ * Tracepoint dynamic linkage handling (callbacks). Hidden visibility: shared
+ * across objects in a module/main executable. The callbacks are used to
+ * control and check if the destructors should be executed.
+ */
+struct lttng_ust_tracepoint_destructors_syms {
+   void (*tracepoint_enable_destructors)(void);
+   void (*tracepoint_disable_destructors)(void);
+   int (*tracepoint_get_destructors_state)(void);
+};
+
+extern struct lttng_ust_tracepoint_destructors_syms 
tracepoint_destructors_syms;
+extern struct lttng_ust_tracepoint_destructors_syms 
*tracepoint_destructors_syms_ptr;
+
+struct lttng_ust_tracepoint_destructors_syms tracepoint_destructors_syms
+   __attribute__((weak, visibility("hidden")));
+struct lttng_ust_tracepoint_destructors_syms *tracepoint_destructors_syms_ptr
+   __attribute__((weak, visibility("hidden")));
+
+static inline void tracepoint_disable_destructors(void)
+{
+   if (!tracepoint_dlopen_ptr)
+   tracepoint_dlopen_ptr = &tracepoint_dlopen;
+   if (!tracepoint_destructors_syms_ptr)
+   tracepoint_destructors_syms_ptr = &tracepoint_destructors_syms;
+   if (tracepoint_dlopen_ptr->liblttngust_handle
+   && 
tracepoint_destructors_syms_ptr->tracepoint_disable_destructors)
+   
tracepoint_destructors_syms_ptr->tracepoint_disable_destructors();
+}
+
+static inline void tracepoint_enable_destructors(void)
+{
+   if (!tracepoint_dlopen_ptr)
+   tracepoint_dlopen_ptr = &tracepoint_dlopen;
+   if (!tracepoint_destructors_syms_ptr)
+   tracepoint_destructors_syms_ptr = &tracepoint_destructors_syms;
+   if (tracepoint_dlopen_ptr->liblttngust_handle
+   && 
tracepoint_destructors_syms_ptr->tracepoint_enable_destructors)
+   
tracepoint_destructors_syms_ptr->tracepoint_enable_destructors();
+}
+
 #ifndef _LGPL_SOURCE
 static inline void lttng_ust_notrace
 __tracepoint__init_urcu_sym(void);
@@ -335,8 +361,11 @@ __tracepoints__destroy(void)
return;
if (!tracepoint_dlopen_ptr)
tracepoint_dlopen_ptr = &tracepoint_dlopen;
-   if (!__tracepoints__disable_destructors
-   && tracepoint_dlopen_ptr->liblttngust_handle
+   if (!tracepoint_destructors_syms_ptr)
+   t