diff mbox series

[RFC,v2,02/13] Reorder pthread_join for easier ordering later on

Message ID 20170918225206.17725-3-jonathan.rajotte-julien@efficios.com
State Superseded, archived
Delegated to: Jérémie Galarneau
Headers show
Series Sessiond teardown overhaul | expand

Commit Message

Jonathan Rajotte Sept. 18, 2017, 10:51 p.m. UTC
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
---
 src/bin/lttng-sessiond/main.c | 96 ++++++++++++++++++++-----------------------
 1 file changed, 45 insertions(+), 51 deletions(-)

Comments

Jérémie Galarneau Dec. 14, 2017, 1:54 a.m. UTC | #1
This patch calls for a better title and description. I see that you are
shuffling the order in which the threads are created and joined.

However, it would help to contrast the current and intended order of
creation/join (and why it is done) in the commit message and comments.

Jérémie

On 18 September 2017 at 18:51, Jonathan Rajotte
<jonathan.rajotte-julien at efficios.com> wrote:
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
>  src/bin/lttng-sessiond/main.c | 96 ++++++++++++++++++++-----------------------
>  1 file changed, 45 insertions(+), 51 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
> index 5d7df744..45c0270e 100644
> --- a/src/bin/lttng-sessiond/main.c
> +++ b/src/bin/lttng-sessiond/main.c
> @@ -6170,15 +6170,26 @@ int main(int argc, char **argv)
>         }
>         notification_thread_running = true;
>
> -       /* Create thread to manage the client socket */
> -       ret = pthread_create(&client_thread, default_pthread_attr(),
> -                       thread_manage_clients, (void *) NULL);
> +       /* 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 clients");
> +               PERROR("pthread_create notify");
>                 retval = -1;
>                 stop_threads();
> -               goto exit_client;
> +               goto exit_apps_notify;
> +       }
> +
> +       /* Create thread to manage application socket */
> +       ret = pthread_create(&apps_thread, default_pthread_attr(),
> +                       thread_manage_apps, (void *) NULL);
> +       if (ret) {
> +               errno = ret;
> +               PERROR("pthread_create apps");
> +               retval = -1;
> +               stop_threads();
> +               goto exit_apps;
>         }
>
>         /* Create thread to dispatch registration */
> @@ -6203,26 +6214,15 @@ int main(int argc, char **argv)
>                 goto exit_reg_apps;
>         }
>
> -       /* Create thread to manage application socket */
> -       ret = pthread_create(&apps_thread, default_pthread_attr(),
> -                       thread_manage_apps, (void *) NULL);
> +       /* Create thread to manage the client socket */
> +       ret = pthread_create(&client_thread, default_pthread_attr(),
> +                       thread_manage_clients, (void *) NULL);
>         if (ret) {
>                 errno = ret;
> -               PERROR("pthread_create apps");
> +               PERROR("pthread_create clients");
>                 retval = -1;
>                 stop_threads();
> -               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;
> +               goto exit_client;
>         }
>
>         /* Create agent registration thread. */
> @@ -6278,12 +6278,12 @@ exit_load_session:
>                 ret = pthread_join(kernel_thread, &status);
>                 if (ret) {
>                         errno = ret;
> -                       PERROR("pthread_join");
> +                       PERROR("pthread_join kernel");
>                         retval = -1;
>                 }
>         }
> -exit_kernel:
>
> +exit_kernel:
>         ret = pthread_join(agent_reg_thread, &status);
>         if (ret) {
>                 errno = ret;
> @@ -6291,51 +6291,45 @@ exit_kernel:
>                 retval = -1;
>         }
>  exit_agent_reg:
> -
> -       ret = pthread_join(apps_notify_thread, &status);
> +       ret = pthread_join(client_thread, &status);
>         if (ret) {
>                 errno = ret;
> -               PERROR("pthread_join apps notify");
> +               PERROR("pthread_join client");
>                 retval = -1;
>         }
> -exit_apps_notify:
>
> +exit_client:
> +       ret = pthread_join(reg_apps_thread, &status);
> +       if (ret) {
> +               errno = ret;
> +               PERROR("pthread_join registration app" );
> +               retval = -1;
> +       }
> +exit_reg_apps:
> +       ret = pthread_join(dispatch_thread, &status);
> +       if (ret) {
> +               errno = ret;
> +               PERROR("pthread_join dispatch");
> +               retval = -1;
> +       }
> +
> +exit_dispatch:
>         ret = pthread_join(apps_thread, &status);
>         if (ret) {
>                 errno = ret;
>                 PERROR("pthread_join apps");
>                 retval = -1;
>         }
> +
>  exit_apps:
> -
> -       ret = pthread_join(reg_apps_thread, &status);
> +       ret = pthread_join(apps_notify_thread, &status);
>         if (ret) {
>                 errno = ret;
> -               PERROR("pthread_join");
> -               retval = -1;
> -       }
> -exit_reg_apps:
> -
> -       /*
> -        * Join dispatch thread after joining reg_apps_thread to ensure
> -        * we don't leak applications in the queue.
> -        */
> -       ret = pthread_join(dispatch_thread, &status);
> -       if (ret) {
> -               errno = ret;
> -               PERROR("pthread_join");
> -               retval = -1;
> -       }
> -exit_dispatch:
> -
> -       ret = pthread_join(client_thread, &status);
> -       if (ret) {
> -               errno = ret;
> -               PERROR("pthread_join");
> +               PERROR("pthread_join apps notify");
>                 retval = -1;
>         }
>
> -exit_client:
> +exit_apps_notify:
>  exit_notification:
>  exit_health:
>  exit_init_data:
> --
> 2.11.0
>
diff mbox series

Patch

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 5d7df744..45c0270e 100644
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -6170,15 +6170,26 @@  int main(int argc, char **argv)
 	}
 	notification_thread_running = true;
 
-	/* Create thread to manage the client socket */
-	ret = pthread_create(&client_thread, default_pthread_attr(),
-			thread_manage_clients, (void *) NULL);
+	/* 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 clients");
+		PERROR("pthread_create notify");
 		retval = -1;
 		stop_threads();
-		goto exit_client;
+		goto exit_apps_notify;
+	}
+
+	/* Create thread to manage application socket */
+	ret = pthread_create(&apps_thread, default_pthread_attr(),
+			thread_manage_apps, (void *) NULL);
+	if (ret) {
+		errno = ret;
+		PERROR("pthread_create apps");
+		retval = -1;
+		stop_threads();
+		goto exit_apps;
 	}
 
 	/* Create thread to dispatch registration */
@@ -6203,26 +6214,15 @@  int main(int argc, char **argv)
 		goto exit_reg_apps;
 	}
 
-	/* Create thread to manage application socket */
-	ret = pthread_create(&apps_thread, default_pthread_attr(),
-			thread_manage_apps, (void *) NULL);
+	/* Create thread to manage the client socket */
+	ret = pthread_create(&client_thread, default_pthread_attr(),
+			thread_manage_clients, (void *) NULL);
 	if (ret) {
 		errno = ret;
-		PERROR("pthread_create apps");
+		PERROR("pthread_create clients");
 		retval = -1;
 		stop_threads();
-		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;
+		goto exit_client;
 	}
 
 	/* Create agent registration thread. */
@@ -6278,12 +6278,12 @@  exit_load_session:
 		ret = pthread_join(kernel_thread, &status);
 		if (ret) {
 			errno = ret;
-			PERROR("pthread_join");
+			PERROR("pthread_join kernel");
 			retval = -1;
 		}
 	}
-exit_kernel:
 
+exit_kernel:
 	ret = pthread_join(agent_reg_thread, &status);
 	if (ret) {
 		errno = ret;
@@ -6291,51 +6291,45 @@  exit_kernel:
 		retval = -1;
 	}
 exit_agent_reg:
-
-	ret = pthread_join(apps_notify_thread, &status);
+	ret = pthread_join(client_thread, &status);
 	if (ret) {
 		errno = ret;
-		PERROR("pthread_join apps notify");
+		PERROR("pthread_join client");
 		retval = -1;
 	}
-exit_apps_notify:
 
+exit_client:
+	ret = pthread_join(reg_apps_thread, &status);
+	if (ret) {
+		errno = ret;
+		PERROR("pthread_join registration app" );
+		retval = -1;
+	}
+exit_reg_apps:
+	ret = pthread_join(dispatch_thread, &status);
+	if (ret) {
+		errno = ret;
+		PERROR("pthread_join dispatch");
+		retval = -1;
+	}
+
+exit_dispatch:
 	ret = pthread_join(apps_thread, &status);
 	if (ret) {
 		errno = ret;
 		PERROR("pthread_join apps");
 		retval = -1;
 	}
+
 exit_apps:
-
-	ret = pthread_join(reg_apps_thread, &status);
+	ret = pthread_join(apps_notify_thread, &status);
 	if (ret) {
 		errno = ret;
-		PERROR("pthread_join");
-		retval = -1;
-	}
-exit_reg_apps:
-
-	/*
-	 * Join dispatch thread after joining reg_apps_thread to ensure
-	 * we don't leak applications in the queue.
-	 */
-	ret = pthread_join(dispatch_thread, &status);
-	if (ret) {
-		errno = ret;
-		PERROR("pthread_join");
-		retval = -1;
-	}
-exit_dispatch:
-
-	ret = pthread_join(client_thread, &status);
-	if (ret) {
-		errno = ret;
-		PERROR("pthread_join");
+		PERROR("pthread_join apps notify");
 		retval = -1;
 	}
 
-exit_client:
+exit_apps_notify:
 exit_notification:
 exit_health:
 exit_init_data: