[lttng-dev] [RFC PATCH v2 10/13] Teardown apps_notify_thread before thread_manage_apps

[Jonathan Rajotte]

RFC:
This is necessary since we use the ust_app_ht_by_notify_sock to
perform the cleanup operation. Since both ust_app_unregister and
ust_app_ust_app_notify_unregister perform a remove of the app on the
ust_ust_app_by_notify_sock but only ust_app_notify_unregister actually
end up performing a close(call_rcu) on the socket.

Other way to do fix this problem?

Could we simply not remove it on a ust_app_unregister? And always defer
to the apps_notify_thread for cleanup?

Update the value in the hash table to -1 and emit a close and remove
from the hash table if the value is -1?

We could also keep a local list of fd in apps_notify_thread and use it for
cleanup instead of relying on ust_ust_app_by_notify_sock.

I'm not sure what is the best/elegant solution here. I am not a fan of
the current solution but it working.

Obviously this commit will be reworded and modified accordingly.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
---
 src/bin/lttng-sessiond/main.c | 55 ++++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 4a2a661f..216d0da6 100644
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -6209,16 +6209,6 @@ int main(int argc, char **argv)
 	}
 	notification_thread_running = true;
 
-	/* Create thread to manage application notify socket */
-	ret = pthread_create(&apps_notify_thread, default_pthread_attr(),
-			ust_thread_manage_notify, (void *) NULL);
-	if (ret) {
-		errno = ret;
-		PERROR("pthread_create notify");
-		retval = -1;
-		stop_threads();
-		goto exit_apps_notify;
-	}
 
 	/* Create thread to manage application socket */
 	ret = pthread_create(&apps_thread, default_pthread_attr(),
@@ -6231,6 +6221,17 @@ int main(int argc, char **argv)
 		goto exit_apps;
 	}
 
+	/* Create thread to manage application notify socket */
+	ret = pthread_create(&apps_notify_thread, default_pthread_attr(),
+			ust_thread_manage_notify, (void *) NULL);
+	if (ret) {
+		errno = ret;
+		PERROR("pthread_create notify");
+		retval = -1;
+		stop_threads();
+		goto exit_apps_notify;
+	}
+
 	/* Create thread to dispatch registration */
 	ret = pthread_create(&dispatch_thread, default_pthread_attr(),
 			thread_dispatch_ust_registration, (void *) NULL);
@@ -6358,20 +6359,6 @@ exit_reg_apps:
 	}
 
 exit_dispatch:
-	/* Instruct the apps thread to quit */
-	ret = notify_thread_pipe(thread_apps_teardown_trigger_pipe[1]);
-	if (ret < 0) {
-		ERR("write error on thread quit pipe");
-	}
-
-	ret = pthread_join(apps_thread, &status);
-	if (ret) {
-		errno = ret;
-		PERROR("pthread_join apps");
-		retval = -1;
-	}
-
-exit_apps:
 	/* Instruct the apps_notify thread to quit */
 	ret = notify_thread_pipe(thread_apps_notify_teardown_trigger_pipe[1]);
 	if (ret < 0) {
@@ -6386,6 +6373,26 @@ exit_apps:
 	}
 
 exit_apps_notify:
+	/*
+	 * The barrier ensure that all previous resources, notify sockets in
+	 * particular, are freed/closed.
+	 */
+	rcu_barrier();
+
+	/* Instruct the apps thread to quit */
+	ret = notify_thread_pipe(thread_apps_teardown_trigger_pipe[1]);
+	if (ret < 0) {
+		ERR("write error on thread quit pipe");
+	}
+
+	ret = pthread_join(apps_thread, &status);
+	if (ret) {
+		errno = ret;
+		PERROR("pthread_join apps");
+		retval = -1;
+	}
+
+exit_apps:
 exit_notification:
 exit_health:
 exit_init_data:
-- 
2.11.0