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

[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 <francis.deslauriers at efficios.com>
---
 include/lttng/tracepoint.h | 82 +++++++++++++++++++++++++++++++++++-----------
 liblttng-ust/tracepoint.c  | 56 +++++++++++++++++++++++++++++++
 2 files changed, 119 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)
+		tracepoint_destructors_syms_ptr = &tracepoint_destructors_syms;
+	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) {
@@ -415,6 +444,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 +454,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->tracepoint_enable_destructors =
+		URCU_FORCE_CAST(void (*)(void),
+				dlsym(tracepoint_dlopen_ptr->liblttngust_handle,
+					"tp_enable_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 +487,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..60f0997 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_refcount > 0, tracepoint destructors are enabled.
+ */
+static int tracepoint_destructors_refcount = 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 refcounting 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,44 @@ 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_{enable,disable}_destructors and tp_get_destructors_state symbols below.
+ */
+void tp_disable_destructors(void)
+{
+	uatomic_dec(&tracepoint_destructors_refcount);
+
+	/*
+	 * Set deprecated destructors execution flag symbol for backward
+	 * compatibility.
+	 */
+	uatomic_set(&__tracepoints__disable_destructors, 1);
+}
+
+void tp_enable_destructors(void)
+{
+	uatomic_inc(&tracepoint_destructors_refcount);
+}
+
+/*
+ * Returns 1 if the destructors are enabled and should be executed.
+ * Returns 0 if the destructors are disabled.
+ */
+int tp_get_destructors_state(void)
+{
+	/*
+	 * Check deprecated destructors exection flag symbol for backward
+	 * compatibility.
+	 */
+	return !__tracepoints__disable_destructors &&
+		uatomic_read(&tracepoint_destructors_refcount) > 0;
+}
-- 
2.7.4