diff mbox series

[lttng-ust] Move symbol preventing unloading of probe providers

Message ID 1536095114-11161-1-git-send-email-francis.deslauriers@efficios.com
State Accepted, archived
Headers show
Series [lttng-ust] Move symbol preventing unloading of probe providers | expand

Commit Message

Francis Deslauriers Sept. 4, 2018, 9:05 p.m. UTC
Issue
diff mbox series

Patch

=====
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 generate 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 only exposed
  by 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 should 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.

Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
---
 include/lttng/tracepoint.h | 108 ++++++++++++++++++++++++++++++++++-----------
 liblttng-ust/tracepoint.c  |  40 +++++++++++++++++
 2 files changed, 123 insertions(+), 25 deletions(-)

diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
index 39f2c4d..9b52bc6 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,39 @@  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();
+	} else {
+		*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 +353,41 @@  __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();
+	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 first;
+	 * use the old method otherwise.
+	 */
+	if (tracepoint_destructors_syms_ptr->tracepoint_get_destructors_state) {
+		if ((tracepoint_destructors_syms_ptr->tracepoint_get_destructors_state)()) {
+			/*
+			 * The tracepoint_get_destructors_state symbol was found with
+			 * dlsym but its returned value is 1 meaning that destructors
+			 * must not be executed.
+			 */
+			return;
 		}
-		memset(tracepoint_dlopen_ptr, 0, sizeof(*tracepoint_dlopen_ptr));
+	} else 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,
@@ -444,8 +501,9 @@  __tracepoints__ptrs_destroy(void)
 		tracepoint_dlopen_ptr = &tracepoint_dlopen;
 	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 2c25d46..5b8c929 100644
--- a/liblttng-ust/tracepoint.c
+++ b/liblttng-ust/tracepoint.c
@@ -53,6 +53,21 @@  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.
+ */
+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 +998,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, 1);
+}
+
+/*
+ * 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);
+}