diff mbox series

[RFC,v2,07/13] Always reply to an inquiring app

Message ID 20170918225206.17725-8-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:52 p.m. UTC
Reply to the app on errors to prevent an app-side receive hang.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
---
 src/bin/lttng-sessiond/ust-app.c | 64 ++++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 28 deletions(-)

Comments

Jérémie Galarneau Dec. 14, 2017, 1:56 a.m. UTC | #1
Looks good to me.

Jérémie

On 18 September 2017 at 18:52, Jonathan Rajotte
<jonathan.rajotte-julien at efficios.com> wrote:
> Reply to the app on errors to prevent an app-side receive hang.
>
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
>  src/bin/lttng-sessiond/ust-app.c | 64 ++++++++++++++++++++++------------------
>  1 file changed, 36 insertions(+), 28 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
> index e79455b0..20ac469b 100644
> --- a/src/bin/lttng-sessiond/ust-app.c
> +++ b/src/bin/lttng-sessiond/ust-app.c
> @@ -5394,13 +5394,13 @@ static int reply_ust_register_channel(int sock, int sobjd, int cobjd,
>                 size_t nr_fields, struct ustctl_field *fields)
>  {
>         int ret, ret_code = 0;
> -       uint32_t chan_id, reg_count;
> -       uint64_t chan_reg_key;
> -       enum ustctl_channel_header type;
> +       uint32_t chan_id = 0, reg_count = 0;
> +       uint64_t chan_reg_key = 0;
> +       enum ustctl_channel_header type = USTCTL_CHANNEL_HEADER_UNKNOWN;
>         struct ust_app *app;
>         struct ust_app_channel *ua_chan;
>         struct ust_app_session *ua_sess;
> -       struct ust_registry_session *registry;
> +       struct ust_registry_session *registry = NULL;
>         struct ust_registry_channel *chan_reg;
>
>         rcu_read_lock();
> @@ -5410,16 +5410,16 @@ static int reply_ust_register_channel(int sock, int sobjd, int cobjd,
>         if (!app) {
>                 DBG("Application socket %d is being torn down. Abort event notify",
>                                 sock);
> -               ret = 0;
> -               goto error_rcu_unlock;
> +               ret_code = -1;
> +               goto reply;
>         }
>
>         /* Lookup channel by UST object descriptor. */
>         ua_chan = find_channel_by_objd(app, cobjd);
>         if (!ua_chan) {
>                 DBG("Application channel is being torn down. Abort event notify");
> -               ret = 0;
> -               goto error_rcu_unlock;
> +               ret_code = -1;
> +               goto reply;
>         }
>
>         assert(ua_chan->session);
> @@ -5429,8 +5429,8 @@ static int reply_ust_register_channel(int sock, int sobjd, int cobjd,
>         registry = get_session_registry(ua_sess);
>         if (!registry) {
>                 DBG("Application session is being torn down. Abort event notify");
> -               ret = 0;
> -               goto error_rcu_unlock;
> +               ret_code = -1;
> +               goto reply;
>         };
>
>         /* Depending on the buffer type, a different channel key is used. */
> @@ -5469,10 +5469,13 @@ static int reply_ust_register_channel(int sock, int sobjd, int cobjd,
>                 ret_code = ust_metadata_channel_statedump(registry, chan_reg);
>                 if (ret_code) {
>                         ERR("Error appending channel metadata (errno = %d)", ret_code);
> -                       goto reply;
> +                       goto unlock_reply;
>                 }
>         }
>
> +unlock_reply:
> +       pthread_mutex_unlock(&registry->lock);
> +
>  reply:
>         DBG3("UST app replying to register channel key %" PRIu64
>                         " with id %u, type: %d, ret: %d", chan_reg_key, chan_id, type,
> @@ -5488,12 +5491,13 @@ reply:
>                 goto error;
>         }
>
> -       /* This channel registry registration is completed. */
> +       if (ret_code < 0) {
> +               goto error;
> +       }
> +
>         chan_reg->register_done = 1;
>
>  error:
> -       pthread_mutex_unlock(&registry->lock);
> -error_rcu_unlock:
>         rcu_read_unlock();
>         free(fields);
>         return ret;
> @@ -5527,16 +5531,16 @@ static int add_event_ust_registry(int sock, int sobjd, int cobjd, char *name,
>         if (!app) {
>                 DBG("Application socket %d is being torn down. Abort event notify",
>                                 sock);
> -               ret = 0;
> -               goto error_rcu_unlock;
> +               ret_code = -1;
> +               goto reply;
>         }
>
>         /* Lookup channel by UST object descriptor. */
>         ua_chan = find_channel_by_objd(app, cobjd);
>         if (!ua_chan) {
>                 DBG("Application channel is being torn down. Abort event notify");
> -               ret = 0;
> -               goto error_rcu_unlock;
> +               ret_code = -1;
> +               goto reply;
>         }
>
>         assert(ua_chan->session);
> @@ -5545,8 +5549,8 @@ static int add_event_ust_registry(int sock, int sobjd, int cobjd, char *name,
>         registry = get_session_registry(ua_sess);
>         if (!registry) {
>                 DBG("Application session is being torn down. Abort event notify");
> -               ret = 0;
> -               goto error_rcu_unlock;
> +               ret_code = -1;
> +               goto reply;
>         }
>
>         if (ua_sess->buffer_type == LTTNG_BUFFER_PER_UID) {
> @@ -5570,6 +5574,9 @@ static int add_event_ust_registry(int sock, int sobjd, int cobjd, char *name,
>         fields = NULL;
>         model_emf_uri = NULL;
>
> +       pthread_mutex_unlock(&registry->lock);
> +
> +reply:
>         /*
>          * The return value is returned to ustctl so in case of an error, the
>          * application can be notified. In case of an error, it's important not to
> @@ -5593,8 +5600,6 @@ static int add_event_ust_registry(int sock, int sobjd, int cobjd, char *name,
>                         name, event_id);
>
>  error:
> -       pthread_mutex_unlock(&registry->lock);
> -error_rcu_unlock:
>         rcu_read_unlock();
>         free(sig);
>         free(fields);
> @@ -5627,8 +5632,9 @@ static int add_enum_ust_registry(int sock, int sobjd, char *name,
>                 /* Return an error since this is not an error */
>                 DBG("Application socket %d is being torn down. Aborting enum registration",
>                                 sock);
> +               ret_code = -1;
>                 free(entries);
> -               goto error_rcu_unlock;
> +               goto reply;
>         }
>
>         /* Lookup session by UST object descriptor. */
> @@ -5637,14 +5643,16 @@ static int add_enum_ust_registry(int sock, int sobjd, char *name,
>                 /* Return an error since this is not an error */
>                 DBG("Application session is being torn down (session not found). Aborting enum registration.");
>                 free(entries);
> -               goto error_rcu_unlock;
> +               ret_code = -1;
> +               goto reply;
>         }
>
>         registry = get_session_registry(ua_sess);
>         if (!registry) {
>                 DBG("Application session is being torn down (registry not found). Aborting enum registration.");
>                 free(entries);
> -               goto error_rcu_unlock;
> +               ret_code = -1;
> +               goto reply;
>         }
>
>         pthread_mutex_lock(&registry->lock);
> @@ -5658,6 +5666,9 @@ static int add_enum_ust_registry(int sock, int sobjd, char *name,
>                         entries, nr_entries, &enum_id);
>         entries = NULL;
>
> +       pthread_mutex_unlock(&registry->lock);
> +
> +reply:
>         /*
>          * The return value is returned to ustctl so in case of an error, the
>          * application can be notified. In case of an error, it's important not to
> @@ -5678,10 +5689,7 @@ static int add_enum_ust_registry(int sock, int sobjd, char *name,
>         }
>
>         DBG3("UST registry enum %s added successfully or already found", name);
> -
>  error:
> -       pthread_mutex_unlock(&registry->lock);
> -error_rcu_unlock:
>         rcu_read_unlock();
>         return ret;
>  }
> --
> 2.11.0
>
diff mbox series

Patch

diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
index e79455b0..20ac469b 100644
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -5394,13 +5394,13 @@  static int reply_ust_register_channel(int sock, int sobjd, int cobjd,
 		size_t nr_fields, struct ustctl_field *fields)
 {
 	int ret, ret_code = 0;
-	uint32_t chan_id, reg_count;
-	uint64_t chan_reg_key;
-	enum ustctl_channel_header type;
+	uint32_t chan_id = 0, reg_count = 0;
+	uint64_t chan_reg_key = 0;
+	enum ustctl_channel_header type = USTCTL_CHANNEL_HEADER_UNKNOWN;
 	struct ust_app *app;
 	struct ust_app_channel *ua_chan;
 	struct ust_app_session *ua_sess;
-	struct ust_registry_session *registry;
+	struct ust_registry_session *registry = NULL;
 	struct ust_registry_channel *chan_reg;
 
 	rcu_read_lock();
@@ -5410,16 +5410,16 @@  static int reply_ust_register_channel(int sock, int sobjd, int cobjd,
 	if (!app) {
 		DBG("Application socket %d is being torn down. Abort event notify",
 				sock);
-		ret = 0;
-		goto error_rcu_unlock;
+		ret_code = -1;
+		goto reply;
 	}
 
 	/* Lookup channel by UST object descriptor. */
 	ua_chan = find_channel_by_objd(app, cobjd);
 	if (!ua_chan) {
 		DBG("Application channel is being torn down. Abort event notify");
-		ret = 0;
-		goto error_rcu_unlock;
+		ret_code = -1;
+		goto reply;
 	}
 
 	assert(ua_chan->session);
@@ -5429,8 +5429,8 @@  static int reply_ust_register_channel(int sock, int sobjd, int cobjd,
 	registry = get_session_registry(ua_sess);
 	if (!registry) {
 		DBG("Application session is being torn down. Abort event notify");
-		ret = 0;
-		goto error_rcu_unlock;
+		ret_code = -1;
+		goto reply;
 	};
 
 	/* Depending on the buffer type, a different channel key is used. */
@@ -5469,10 +5469,13 @@  static int reply_ust_register_channel(int sock, int sobjd, int cobjd,
 		ret_code = ust_metadata_channel_statedump(registry, chan_reg);
 		if (ret_code) {
 			ERR("Error appending channel metadata (errno = %d)", ret_code);
-			goto reply;
+			goto unlock_reply;
 		}
 	}
 
+unlock_reply:
+	pthread_mutex_unlock(&registry->lock);
+
 reply:
 	DBG3("UST app replying to register channel key %" PRIu64
 			" with id %u, type: %d, ret: %d", chan_reg_key, chan_id, type,
@@ -5488,12 +5491,13 @@  reply:
 		goto error;
 	}
 
-	/* This channel registry registration is completed. */
+	if (ret_code < 0) {
+		goto error;
+	}
+
 	chan_reg->register_done = 1;
 
 error:
-	pthread_mutex_unlock(&registry->lock);
-error_rcu_unlock:
 	rcu_read_unlock();
 	free(fields);
 	return ret;
@@ -5527,16 +5531,16 @@  static int add_event_ust_registry(int sock, int sobjd, int cobjd, char *name,
 	if (!app) {
 		DBG("Application socket %d is being torn down. Abort event notify",
 				sock);
-		ret = 0;
-		goto error_rcu_unlock;
+		ret_code = -1;
+		goto reply;
 	}
 
 	/* Lookup channel by UST object descriptor. */
 	ua_chan = find_channel_by_objd(app, cobjd);
 	if (!ua_chan) {
 		DBG("Application channel is being torn down. Abort event notify");
-		ret = 0;
-		goto error_rcu_unlock;
+		ret_code = -1;
+		goto reply;
 	}
 
 	assert(ua_chan->session);
@@ -5545,8 +5549,8 @@  static int add_event_ust_registry(int sock, int sobjd, int cobjd, char *name,
 	registry = get_session_registry(ua_sess);
 	if (!registry) {
 		DBG("Application session is being torn down. Abort event notify");
-		ret = 0;
-		goto error_rcu_unlock;
+		ret_code = -1;
+		goto reply;
 	}
 
 	if (ua_sess->buffer_type == LTTNG_BUFFER_PER_UID) {
@@ -5570,6 +5574,9 @@  static int add_event_ust_registry(int sock, int sobjd, int cobjd, char *name,
 	fields = NULL;
 	model_emf_uri = NULL;
 
+	pthread_mutex_unlock(&registry->lock);
+
+reply:
 	/*
 	 * The return value is returned to ustctl so in case of an error, the
 	 * application can be notified. In case of an error, it's important not to
@@ -5593,8 +5600,6 @@  static int add_event_ust_registry(int sock, int sobjd, int cobjd, char *name,
 			name, event_id);
 
 error:
-	pthread_mutex_unlock(&registry->lock);
-error_rcu_unlock:
 	rcu_read_unlock();
 	free(sig);
 	free(fields);
@@ -5627,8 +5632,9 @@  static int add_enum_ust_registry(int sock, int sobjd, char *name,
 		/* Return an error since this is not an error */
 		DBG("Application socket %d is being torn down. Aborting enum registration",
 				sock);
+		ret_code = -1;
 		free(entries);
-		goto error_rcu_unlock;
+		goto reply;
 	}
 
 	/* Lookup session by UST object descriptor. */
@@ -5637,14 +5643,16 @@  static int add_enum_ust_registry(int sock, int sobjd, char *name,
 		/* Return an error since this is not an error */
 		DBG("Application session is being torn down (session not found). Aborting enum registration.");
 		free(entries);
-		goto error_rcu_unlock;
+		ret_code = -1;
+		goto reply;
 	}
 
 	registry = get_session_registry(ua_sess);
 	if (!registry) {
 		DBG("Application session is being torn down (registry not found). Aborting enum registration.");
 		free(entries);
-		goto error_rcu_unlock;
+		ret_code = -1;
+		goto reply;
 	}
 
 	pthread_mutex_lock(&registry->lock);
@@ -5658,6 +5666,9 @@  static int add_enum_ust_registry(int sock, int sobjd, char *name,
 			entries, nr_entries, &enum_id);
 	entries = NULL;
 
+	pthread_mutex_unlock(&registry->lock);
+
+reply:
 	/*
 	 * The return value is returned to ustctl so in case of an error, the
 	 * application can be notified. In case of an error, it's important not to
@@ -5678,10 +5689,7 @@  static int add_enum_ust_registry(int sock, int sobjd, char *name,
 	}
 
 	DBG3("UST registry enum %s added successfully or already found", name);
-
 error:
-	pthread_mutex_unlock(&registry->lock);
-error_rcu_unlock:
 	rcu_read_unlock();
 	return ret;
 }