Move symbol preventing unloading of probe providers

Issue
=====
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. Since the
unregistration of the probes is postponed, event will be generated if
the callsite is executed even though the probes should not be loaded.

Solution
========
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 and use dlsym to get handles on functions
that disable and get the state of the destructors.

Version compatibility
=====================
- This change is backward compatible with UST applications and libraries
  built on lttng-ust version before 2.11. Those applications will use
  the __tracepoints__disable_destructors symbol that is now exposed
  as a weak symbol in the liblttng-ust-tracepoint.so library. This
  symbol is alway checked in 2.11 in case an old app is running.

- Applications built with this change will also work in older versions
  of lttng-ust as there is a check to see if the new destructor state
  checking method should be used, if it is not we fallback to a
  compatibility method. To ensure compatibility in this case, we also
  look up and keep up-to-date the __tracepoints__disable_destructors
  value using the dlopen-dlsym combo.

- A mix of applications/probes builds in part against 2.10 and 2.11
  also work. When setting the destructor state from a binary built
  against 2.11 headers, both old/new states are set, so a binary built
  against 2.10 will correctly see the old state. When querying the state
  from a binary built against 2.11 headers, both old and new states are
  queried, so if the state has been set from a binary built against
  2.10 headers, the old state will be set.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
index 39f2c4db752342f14c5bd8206174093595ebacec..d77a2fb298c40f99c7fd290eb48eda48d36bcae9 100644 (file)
--- 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,37 @@ 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 {
+       int *old_tracepoint_disable_destructors;
+       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();
+       *tracepoint_destructors_syms_ptr->old_tracepoint_disable_destructors = 1;
+}
+
 #ifndef _LGPL_SOURCE
 static inline void lttng_ust_notrace
 __tracepoint__init_urcu_sym(void);
@@ -335,16 +351,43 @@ __tracepoints__destroy(void)
                return;
        if (!tracepoint_dlopen_ptr)
                tracepoint_dlopen_ptr = &tracepoint_dlopen;
-       if (!__tracepoints__disable_destructors
-                       && tracepoint_dlopen_ptr->liblttngust_handle
-                       && !__tracepoint_ptrs_registered) {
-               ret = dlclose(tracepoint_dlopen_ptr->liblttngust_handle);
-               if (ret) {
-                       fprintf(stderr, "Error (%d) in dlclose\n", ret);
-                       abort();
-               }
-               memset(tracepoint_dlopen_ptr, 0, sizeof(*tracepoint_dlopen_ptr));
+       if (!tracepoint_destructors_syms_ptr)
+               tracepoint_destructors_syms_ptr = &tracepoint_destructors_syms;
+       if (!tracepoint_dlopen_ptr->liblttngust_handle)
+               return;
+       if (__tracepoint_ptrs_registered)
+               return;
+       /*
+        * Lookup if destructors must be executed using the new method.
+        */
+       if (tracepoint_destructors_syms_ptr->tracepoint_get_destructors_state
+               && !tracepoint_destructors_syms_ptr->tracepoint_get_destructors_state()) {
+               /*
+                * The tracepoint_get_destructors_state symbol was found with
+                * dlsym but its returned value is 0 meaning that destructors
+                * must not be executed.
+                */
+               return;
+       }
+       /*
+        * Lookup if destructors must be executed using the old method.
+        */
+       if (tracepoint_destructors_syms_ptr->old_tracepoint_disable_destructors
+               && *tracepoint_destructors_syms_ptr->old_tracepoint_disable_destructors) {
+               /*
+                * The old_tracepoint_disable_destructors symbol was found with
+                * dlsym but its value is 1 meaning that destructors must not
+                * be executed.
+                */
+               return;
        }
+
+       ret = dlclose(tracepoint_dlopen_ptr->liblttngust_handle);
+       if (ret) {
+               fprintf(stderr, "Error (%d) in dlclose\n", ret);
+               abort();
+       }
+       memset(tracepoint_dlopen_ptr, 0, sizeof(*tracepoint_dlopen_ptr));
 }
 
 #ifdef TRACEPOINT_DEFINE
@@ -415,6 +458,8 @@ __tracepoints__ptrs_init(void)
                        dlopen("liblttng-ust-tracepoint.so.0", RTLD_NOW | RTLD_GLOBAL);
        if (!tracepoint_dlopen_ptr->liblttngust_handle)
                return;
+       if (!tracepoint_destructors_syms_ptr)
+               tracepoint_destructors_syms_ptr = &tracepoint_destructors_syms;
        tracepoint_dlopen_ptr->tracepoint_register_lib =
                URCU_FORCE_CAST(int (*)(struct lttng_ust_tracepoint * const *, int),
                                dlsym(tracepoint_dlopen_ptr->liblttngust_handle,
@@ -423,6 +468,18 @@ __tracepoints__ptrs_init(void)
                URCU_FORCE_CAST(int (*)(struct lttng_ust_tracepoint * const *),
                                dlsym(tracepoint_dlopen_ptr->liblttngust_handle,
                                        "tracepoint_unregister_lib"));
+       tracepoint_destructors_syms_ptr->old_tracepoint_disable_destructors =
+               URCU_FORCE_CAST(int *,
+                               dlsym(tracepoint_dlopen_ptr->liblttngust_handle,
+                                       "__tracepoints__disable_destructors"));
+       tracepoint_destructors_syms_ptr->tracepoint_disable_destructors =
+               URCU_FORCE_CAST(void (*)(void),
+                               dlsym(tracepoint_dlopen_ptr->liblttngust_handle,
+                                       "tp_disable_destructors"));
+       tracepoint_destructors_syms_ptr->tracepoint_get_destructors_state =
+               URCU_FORCE_CAST(int (*)(void),
+                               dlsym(tracepoint_dlopen_ptr->liblttngust_handle,
+                                       "tp_get_destructors_state"));
        __tracepoint__init_urcu_sym();
        if (tracepoint_dlopen_ptr->tracepoint_register_lib) {
                tracepoint_dlopen_ptr->tracepoint_register_lib(__start___tracepoints_ptrs,
@@ -442,10 +499,13 @@ __tracepoints__ptrs_destroy(void)
                return;
        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->tracepoint_unregister_lib)
                tracepoint_dlopen_ptr->tracepoint_unregister_lib(__start___tracepoints_ptrs);
-       if (!__tracepoints__disable_destructors
-                       && tracepoint_dlopen_ptr->liblttngust_handle
+       if (tracepoint_dlopen_ptr->liblttngust_handle
+                       && tracepoint_destructors_syms_ptr->tracepoint_get_destructors_state
+                       && tracepoint_destructors_syms_ptr->tracepoint_get_destructors_state()
                        && !__tracepoint_ptrs_registered) {
                ret = dlclose(tracepoint_dlopen_ptr->liblttngust_handle);
                if (ret) {

diff --git a/liblttng-ust/tracepoint.c b/liblttng-ust/tracepoint.c
index 2c25d46250b603c6ed3e5c987f24aae905ef482a..efd95bc1233ef07086e3a889166e40eca3b3d4a7 100644 (file)
--- a/liblttng-ust/tracepoint.c
+++ b/liblttng-ust/tracepoint.c
@@ -53,6 +53,22 @@ struct {
 /* Set to 1 to enable tracepoint debug output */
 static const int tracepoint_debug;
 static int initialized;
+
+/*
+ * If tracepoint_destructors_state = 1, tracepoint destructors are
+ * enabled. They are disabled otherwise.
+ */
+static int tracepoint_destructors_state = 1;
+
+/*
+ * Expose the now deprecated symbol __tracepoints__disable_destructors for
+ * backward compatibility of applications built against old versions of
+ * lttng-ust. We need to keep __tracepoints__disable_destructors up to date
+ * within the new destructor disabling API because old applications read this
+ * symbol directly.
+ */
+int __tracepoints__disable_destructors __attribute__((weak));
+
 static void (*new_tracepoint_cb)(struct lttng_ust_tracepoint *);
 
 /*
@@ -983,3 +999,28 @@ void *tp_rcu_dereference_sym_bp(void *p)
 {
        return rcu_dereference_bp(p);
 }
+
+/*
+ * Programs that have threads that survive after they exit, and therefore call
+ * library destructors, should disable the tracepoint destructors by calling
+ * tp_disable_destructors(). This will leak the tracepoint
+ * instrumentation library shared object, leaving its teardown to the operating
+ * system process teardown.
+ *
+ * To access and/or modify this value, users need to use a combination of
+ * dlopen(3) and dlsym(3) to get an handle on the
+ * tp_disable_destructors and tp_get_destructors_state symbols below.
+ */
+void tp_disable_destructors(void)
+{
+       uatomic_set(&tracepoint_destructors_state, 0);
+}
+
+/*
+ * Returns 1 if the destructors are enabled and should be executed.
+ * Returns 0 if the destructors are disabled.
+ */
+int tp_get_destructors_state(void)
+{
+       return uatomic_read(&tracepoint_destructors_state);
+}