From d4769e1418514c6320ff16b2585562308f108c90 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Sun, 16 Aug 2015 17:10:22 -0400 Subject: [PATCH] Fix: sessiond ust-app session teardown race MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Add a deleted flag within the ust app session which is raised (with ust app session lock held) at delete, and checked within each RCU traversal, again with ust app session lock held. This takes care of races between teardown of an application (unregister) and execution of commands which are accessing the app session concurrently. Signed-off-by: Mathieu Desnoyers Signed-off-by: Jérémie Galarneau --- src/bin/lttng-sessiond/cmd.c | 2 +- src/bin/lttng-sessiond/ust-app.c | 59 ++++++++++++++++++++++++++++++-- src/bin/lttng-sessiond/ust-app.h | 2 ++ 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c index b6dbccc82..217d45a25 100644 --- a/src/bin/lttng-sessiond/cmd.c +++ b/src/bin/lttng-sessiond/cmd.c @@ -3154,7 +3154,7 @@ int cmd_snapshot_record(struct ltt_session *session, int ret = LTTNG_OK; unsigned int use_tmp_output = 0; struct snapshot_output tmp_output; - unsigned int nb_streams, snapshot_success = 0; + unsigned int snapshot_success = 0; assert(session); assert(output); diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c index 96460ac87..22f25da35 100644 --- a/src/bin/lttng-sessiond/ust-app.c +++ b/src/bin/lttng-sessiond/ust-app.c @@ -587,7 +587,6 @@ static int push_metadata(struct ust_registry_session *registry, return 0; error: -end: pthread_mutex_unlock(®istry->lock); return ret_val; } @@ -677,6 +676,9 @@ void delete_ust_app_session(int sock, struct ust_app_session *ua_sess, pthread_mutex_lock(&ua_sess->lock); + assert(!ua_sess->deleted); + ua_sess->deleted = true; + registry = get_session_registry(ua_sess); if (registry) { /* Push metadata for application before freeing the application. */ @@ -3134,6 +3136,11 @@ void ust_app_unregister(int sock) */ pthread_mutex_lock(&ua_sess->lock); + if (ua_sess->deleted) { + pthread_mutex_unlock(&ua_sess->lock); + continue; + } + /* * Normally, this is done in the delete session process which is * executed in the call rcu below. However, upon registration we can't @@ -3699,6 +3706,12 @@ int ust_app_create_channel_glb(struct ltt_ust_session *usess, assert(ua_sess); pthread_mutex_lock(&ua_sess->lock); + + if (ua_sess->deleted) { + pthread_mutex_unlock(&ua_sess->lock); + continue; + } + if (!strncmp(uchan->name, DEFAULT_METADATA_NAME, sizeof(uchan->name))) { copy_channel_attr_to_ustctl(&ua_sess->metadata_attr, &uchan->attr); @@ -3768,6 +3781,11 @@ int ust_app_enable_event_glb(struct ltt_ust_session *usess, pthread_mutex_lock(&ua_sess->lock); + if (ua_sess->deleted) { + pthread_mutex_unlock(&ua_sess->lock); + continue; + } + /* Lookup channel in the ust app session */ lttng_ht_lookup(ua_sess->channels, (void *)uchan->name, &uiter); ua_chan_node = lttng_ht_iter_get_node_str(&uiter); @@ -3834,6 +3852,12 @@ int ust_app_create_event_glb(struct ltt_ust_session *usess, } pthread_mutex_lock(&ua_sess->lock); + + if (ua_sess->deleted) { + pthread_mutex_unlock(&ua_sess->lock); + continue; + } + /* Lookup channel in the ust app session */ lttng_ht_lookup(ua_sess->channels, (void *)uchan->name, &uiter); ua_chan_node = lttng_ht_iter_get_node_str(&uiter); @@ -3885,6 +3909,11 @@ int ust_app_start_trace(struct ltt_ust_session *usess, struct ust_app *app) pthread_mutex_lock(&ua_sess->lock); + if (ua_sess->deleted) { + pthread_mutex_unlock(&ua_sess->lock); + goto end; + } + /* Upon restart, we skip the setup, already done */ if (ua_sess->started) { goto skip_setup; @@ -3985,6 +4014,11 @@ int ust_app_stop_trace(struct ltt_ust_session *usess, struct ust_app *app) pthread_mutex_lock(&ua_sess->lock); + if (ua_sess->deleted) { + pthread_mutex_unlock(&ua_sess->lock); + goto end_no_session; + } + /* * If started = 0, it means that stop trace has been called for a session * that was never started. It's possible since we can have a fail start @@ -4065,6 +4099,10 @@ int ust_app_flush_app_session(struct ust_app *app, pthread_mutex_lock(&ua_sess->lock); + if (ua_sess->deleted) { + goto end_deleted; + } + health_code_update(); /* Flushing buffers */ @@ -4094,6 +4132,7 @@ int ust_app_flush_app_session(struct ust_app *app, health_code_update(); +end_deleted: pthread_mutex_unlock(&ua_sess->lock); end_not_compatible: @@ -4174,7 +4213,6 @@ int ust_app_flush_session(struct ltt_ust_session *usess) break; } -end_no_session: rcu_read_unlock(); health_code_update(); return ret; @@ -4348,6 +4386,11 @@ void ust_app_global_update(struct ltt_ust_session *usess, int sock) pthread_mutex_lock(&ua_sess->lock); + if (ua_sess->deleted) { + pthread_mutex_unlock(&ua_sess->lock); + goto error; + } + /* * We can iterate safely here over all UST app session since the create ust * app session above made a shadow copy of the UST global domain from the @@ -4441,6 +4484,12 @@ int ust_app_add_ctx_channel_glb(struct ltt_ust_session *usess, } pthread_mutex_lock(&ua_sess->lock); + + if (ua_sess->deleted) { + pthread_mutex_unlock(&ua_sess->lock); + continue; + } + /* Lookup channel in the ust app session */ lttng_ht_lookup(ua_sess->channels, (void *)uchan->name, &uiter); ua_chan_node = lttng_ht_iter_get_node_str(&uiter); @@ -4499,6 +4548,12 @@ int ust_app_enable_event_pid(struct ltt_ust_session *usess, } pthread_mutex_lock(&ua_sess->lock); + + if (ua_sess->deleted) { + ret = 0; + goto end_unlock; + } + /* Lookup channel in the ust app session */ lttng_ht_lookup(ua_sess->channels, (void *)uchan->name, &iter); ua_chan_node = lttng_ht_iter_get_node_str(&iter); diff --git a/src/bin/lttng-sessiond/ust-app.h b/src/bin/lttng-sessiond/ust-app.h index a3440e21c..78381dd6c 100644 --- a/src/bin/lttng-sessiond/ust-app.h +++ b/src/bin/lttng-sessiond/ust-app.h @@ -183,6 +183,8 @@ struct ust_app_session { int started; /* allows detection of start vs restart. */ int handle; /* used has unique identifier for app session */ + bool deleted; /* Session deleted flag. Check with lock held. */ + /* * Tracing session ID. Multiple ust app session can have the same tracing * session id making this value NOT unique to the object. -- 2.34.1