Fix: hash table cleanup call_rcu deadlock
[lttng-tools.git] / src / bin / lttng-sessiond / ust-app.c
index fdcad1c304ab0c717fb3f619b812de6d17433b31..37f6442156ff03a8724ef62137e66b1548544f30 100644 (file)
@@ -38,6 +38,7 @@
 #include "ust-app.h"
 #include "ust-consumer.h"
 #include "ust-ctl.h"
+#include "utils.h"
 
 /* Next available channel key. */
 static unsigned long next_channel_key;
@@ -305,6 +306,23 @@ void delete_ust_app_stream(int sock, struct ust_app_stream *stream)
        free(stream);
 }
 
+/*
+ * We need to execute ht_destroy outside of RCU read-side critical
+ * section and outside of call_rcu thread, so we postpone its execution
+ * using ht_cleanup_push. It is simpler than to change the semantic of
+ * the many callers of delete_ust_app_session().
+ */
+static
+void delete_ust_app_channel_rcu(struct rcu_head *head)
+{
+       struct ust_app_channel *ua_chan =
+               caa_container_of(head, struct ust_app_channel, rcu_head);
+
+       ht_cleanup_push(ua_chan->ctx);
+       ht_cleanup_push(ua_chan->events);
+       free(ua_chan);
+}
+
 /*
  * Delete ust app channel safely. RCU read lock must be held before calling
  * this function.
@@ -336,7 +354,6 @@ void delete_ust_app_channel(int sock, struct ust_app_channel *ua_chan,
                assert(!ret);
                delete_ust_app_ctx(sock, ua_ctx);
        }
-       lttng_ht_destroy(ua_chan->ctx);
 
        /* Wipe events */
        cds_lfht_for_each_entry(ua_chan->events->ht, &iter.iter, ua_event,
@@ -345,7 +362,6 @@ void delete_ust_app_channel(int sock, struct ust_app_channel *ua_chan,
                assert(!ret);
                delete_ust_app_event(sock, ua_event);
        }
-       lttng_ht_destroy(ua_chan->events);
 
        /* Wipe and free registry from session registry. */
        registry = get_session_registry(ua_chan->session);
@@ -365,11 +381,14 @@ void delete_ust_app_channel(int sock, struct ust_app_channel *ua_chan,
                lttng_fd_put(LTTNG_FD_APPS, 1);
                free(ua_chan->obj);
        }
-       free(ua_chan);
+       call_rcu(&ua_chan->rcu_head, delete_ust_app_channel_rcu);
 }
 
 /*
- * Push metadata to consumer socket. The socket lock MUST be acquired.
+ * Push metadata to consumer socket.
+ *
+ * The socket lock MUST be acquired.
+ * The ust app session lock MUST be acquired.
  *
  * On success, return the len of metadata pushed or else a negative value.
  */
@@ -383,8 +402,19 @@ ssize_t ust_app_push_metadata(struct ust_registry_session *registry,
 
        assert(registry);
        assert(socket);
-       /* Should never be 0 which is the initial state. */
-       assert(registry->metadata_key);
+
+       /*
+        * On a push metadata error either the consumer is dead or the metadata
+        * channel has been destroyed because its endpoint might have died (e.g:
+        * relayd). If so, the metadata closed flag is set to 1 so we deny pushing
+        * metadata again which is not valid anymore on the consumer side.
+        *
+        * The ust app session mutex locked allows us to make this check without
+        * the registry lock.
+        */
+       if (registry->metadata_closed) {
+               return -EPIPE;
+       }
 
        pthread_mutex_lock(&registry->lock);
 
@@ -459,7 +489,7 @@ static int push_metadata(struct ust_registry_session *registry,
         */
        if (!registry->metadata_key) {
                ret_val = 0;
-               goto error_rcu_unlock;
+               goto end_rcu_unlock;
        }
 
        /* Get consumer socket to use to push the metadata.*/
@@ -492,6 +522,13 @@ static int push_metadata(struct ust_registry_session *registry,
        return 0;
 
 error_rcu_unlock:
+       /*
+        * On error, flag the registry that the metadata is closed. We were unable
+        * to push anything and this means that either the consumer is not
+        * responding or the metadata cache has been destroyed on the consumer.
+        */
+       registry->metadata_closed = 1;
+end_rcu_unlock:
        rcu_read_unlock();
        return ret_val;
 }
@@ -517,7 +554,7 @@ static int close_metadata(struct ust_registry_session *registry,
 
        if (!registry->metadata_key || registry->metadata_closed) {
                ret = 0;
-               goto error;
+               goto end;
        }
 
        /* Get consumer socket to use to push the metadata.*/
@@ -533,14 +570,34 @@ static int close_metadata(struct ust_registry_session *registry,
                goto error;
        }
 
-       /* Metadata successfully closed. Flag the registry. */
-       registry->metadata_closed = 1;
-
 error:
+       /*
+        * Metadata closed. Even on error this means that the consumer is not
+        * responding or not found so either way a second close should NOT be emit
+        * for this registry.
+        */
+       registry->metadata_closed = 1;
+end:
        rcu_read_unlock();
        return ret;
 }
 
+/*
+ * We need to execute ht_destroy outside of RCU read-side critical
+ * section and outside of call_rcu thread, so we postpone its execution
+ * using ht_cleanup_push. It is simpler than to change the semantic of
+ * the many callers of delete_ust_app_session().
+ */
+static
+void delete_ust_app_session_rcu(struct rcu_head *head)
+{
+       struct ust_app_session *ua_sess =
+               caa_container_of(head, struct ust_app_session, rcu_head);
+
+       ht_cleanup_push(ua_sess->channels);
+       free(ua_sess);
+}
+
 /*
  * Delete ust app session safely. RCU read lock must be held before calling
  * this function.
@@ -556,16 +613,21 @@ void delete_ust_app_session(int sock, struct ust_app_session *ua_sess,
 
        assert(ua_sess);
 
+       pthread_mutex_lock(&ua_sess->lock);
+
        registry = get_session_registry(ua_sess);
-       if (registry) {
+       if (registry && !registry->metadata_closed) {
                /* Push metadata for application before freeing the application. */
                (void) push_metadata(registry, ua_sess->consumer);
 
                /*
                 * Don't ask to close metadata for global per UID buffers. Close
-                * metadata only on destroy trace session in this case.
+                * metadata only on destroy trace session in this case. Also, the
+                * previous push metadata could have flag the metadata registry to
+                * close so don't send a close command if closed.
                 */
-               if (ua_sess->buffer_type != LTTNG_BUFFER_PER_UID) {
+               if (ua_sess->buffer_type != LTTNG_BUFFER_PER_UID &&
+                               !registry->metadata_closed) {
                        /* And ask to close it for this session registry. */
                        (void) close_metadata(registry, ua_sess->consumer);
                }
@@ -577,7 +639,6 @@ void delete_ust_app_session(int sock, struct ust_app_session *ua_sess,
                assert(!ret);
                delete_ust_app_channel(sock, ua_chan, app);
        }
-       lttng_ht_destroy(ua_sess->channels);
 
        /* In case of per PID, the registry is kept in the session. */
        if (ua_sess->buffer_type == LTTNG_BUFFER_PER_PID) {
@@ -595,12 +656,16 @@ void delete_ust_app_session(int sock, struct ust_app_session *ua_sess,
                                        sock, ret);
                }
        }
-       free(ua_sess);
+       pthread_mutex_unlock(&ua_sess->lock);
+
+       call_rcu(&ua_sess->rcu_head, delete_ust_app_session_rcu);
 }
 
 /*
  * Delete a traceable application structure from the global list. Never call
  * this function outside of a call_rcu call.
+ *
+ * RCU read side lock should _NOT_ be held when calling this function.
  */
 static
 void delete_ust_app(struct ust_app *app)
@@ -608,21 +673,21 @@ void delete_ust_app(struct ust_app *app)
        int ret, sock;
        struct ust_app_session *ua_sess, *tmp_ua_sess;
 
-       rcu_read_lock();
-
        /* Delete ust app sessions info */
        sock = app->sock;
        app->sock = -1;
 
-       lttng_ht_destroy(app->sessions);
-
        /* Wipe sessions */
        cds_list_for_each_entry_safe(ua_sess, tmp_ua_sess, &app->teardown_head,
                        teardown_node) {
                /* Free every object in the session and the session. */
+               rcu_read_lock();
                delete_ust_app_session(sock, ua_sess, app);
+               rcu_read_unlock();
        }
-       lttng_ht_destroy(app->ust_objd);
+
+       ht_cleanup_push(app->sessions);
+       ht_cleanup_push(app->ust_objd);
 
        /*
         * Wait until we have deleted the application from the sock hash table
@@ -645,8 +710,6 @@ void delete_ust_app(struct ust_app *app)
 
        DBG2("UST app pid %d deleted", app->pid);
        free(app);
-
-       rcu_read_unlock();
 }
 
 /*
@@ -1298,6 +1361,9 @@ static void shadow_copy_channel(struct ust_app_channel *ua_chan,
        strncpy(ua_chan->name, uchan->name, sizeof(ua_chan->name));
        ua_chan->name[sizeof(ua_chan->name) - 1] = '\0';
 
+       ua_chan->tracefile_size = uchan->tracefile_size;
+       ua_chan->tracefile_count = uchan->tracefile_count;
+
        /* Copy event attributes since the layout is different. */
        ua_chan->attr.subbuf_size = uchan->attr.subbuf_size;
        ua_chan->attr.num_subbuf = uchan->attr.num_subbuf;
@@ -1378,7 +1444,7 @@ static void shadow_copy_session(struct ust_app_session *ua_sess,
        switch (ua_sess->buffer_type) {
        case LTTNG_BUFFER_PER_PID:
                ret = snprintf(ua_sess->path, sizeof(ua_sess->path),
-                               DEFAULT_UST_TRACE_PID_PATH "/%s-%d-%s/", app->name, app->pid,
+                               DEFAULT_UST_TRACE_PID_PATH "/%s-%d-%s", app->name, app->pid,
                                datetime);
                break;
        case LTTNG_BUFFER_PER_UID:
@@ -1499,7 +1565,9 @@ static int setup_buffer_reg_pid(struct ust_app_session *ua_sess,
        ret = ust_registry_session_init(&reg_pid->registry->reg.ust, app,
                        app->bits_per_long, app->uint8_t_alignment,
                        app->uint16_t_alignment, app->uint32_t_alignment,
-                       app->uint64_t_alignment, app->long_alignment, app->byte_order);
+                       app->uint64_t_alignment, app->long_alignment,
+                       app->byte_order, app->version.major,
+                       app->version.minor);
        if (ret < 0) {
                goto error;
        }
@@ -1550,10 +1618,12 @@ static int setup_buffer_reg_uid(struct ltt_ust_session *usess,
        }
 
        /* Initialize registry. */
-       ret = ust_registry_session_init(&reg_uid->registry->reg.ust, app,
+       ret = ust_registry_session_init(&reg_uid->registry->reg.ust, NULL,
                        app->bits_per_long, app->uint8_t_alignment,
                        app->uint16_t_alignment, app->uint32_t_alignment,
-                       app->uint64_t_alignment, app->long_alignment, app->byte_order);
+                       app->uint64_t_alignment, app->long_alignment,
+                       app->byte_order, app->version.major,
+                       app->version.minor);
        if (ret < 0) {
                goto error;
        }
@@ -2146,6 +2216,7 @@ static int send_channel_uid_to_ust(struct buffer_reg_channel *reg_chan,
 
                ret = ust_consumer_send_stream_to_ust(app, ua_chan, &stream);
                if (ret < 0) {
+                       (void) release_ust_app_stream(-1, &stream);
                        goto error_stream_unlock;
                }
 
@@ -2208,6 +2279,14 @@ static int create_channel_per_uid(struct ust_app *app,
                ret = do_consumer_create_channel(usess, ua_sess, ua_chan,
                                app->bits_per_long, reg_uid->registry->reg.ust);
                if (ret < 0) {
+                       /*
+                        * Let's remove the previously created buffer registry channel so
+                        * it's not visible anymore in the session registry.
+                        */
+                       ust_registry_channel_del_free(reg_uid->registry->reg.ust,
+                                       ua_chan->tracing_channel_id);
+                       buffer_reg_channel_remove(reg_uid->registry, reg_chan);
+                       buffer_reg_channel_destroy(reg_chan, LTTNG_DOMAIN_UST);
                        goto error;
                }
 
@@ -2340,7 +2419,7 @@ error:
  * Create UST app channel and create it on the tracer. Set ua_chanp of the
  * newly created channel if not NULL.
  *
- * Called with UST app session lock held.
+ * Called with UST app session lock and RCU read-side lock held.
  *
  * Return 0 on success or else a negative value.
  */
@@ -2366,7 +2445,7 @@ static int create_ust_app_channel(struct ust_app_session *ua_sess,
        if (ua_chan == NULL) {
                /* Only malloc can fail here */
                ret = -ENOMEM;
-               goto error;
+               goto error_alloc;
        }
        shadow_copy_channel(ua_chan, uchan);
 
@@ -2394,6 +2473,7 @@ end:
 
 error:
        delete_ust_app_channel(ua_chan->is_sent ? app->sock : -1, ua_chan, app);
+error_alloc:
        return ret;
 }
 
@@ -2470,8 +2550,8 @@ static int create_ust_app_metadata(struct ust_app_session *ua_sess,
        registry = get_session_registry(ua_sess);
        assert(registry);
 
-       /* Metadata already exists for this registry. */
-       if (registry->metadata_key) {
+       /* Metadata already exists for this registry or it was closed previously */
+       if (registry->metadata_key || registry->metadata_closed) {
                ret = 0;
                goto error;
        }
@@ -2489,8 +2569,8 @@ static int create_ust_app_metadata(struct ust_app_session *ua_sess,
                metadata->attr.overwrite = DEFAULT_CHANNEL_OVERWRITE;
                metadata->attr.subbuf_size = default_get_metadata_subbuf_size();
                metadata->attr.num_subbuf = DEFAULT_METADATA_SUBBUF_NUM;
-               metadata->attr.switch_timer_interval = DEFAULT_UST_CHANNEL_SWITCH_TIMER;
-               metadata->attr.read_timer_interval = DEFAULT_UST_CHANNEL_READ_TIMER;
+               metadata->attr.switch_timer_interval = DEFAULT_METADATA_SWITCH_TIMER;
+               metadata->attr.read_timer_interval = DEFAULT_METADATA_READ_TIMER;
                metadata->attr.output = LTTNG_UST_MMAP;
                metadata->attr.type = LTTNG_UST_CHAN_METADATA;
        } else {
@@ -2499,13 +2579,6 @@ static int create_ust_app_metadata(struct ust_app_session *ua_sess,
                metadata->attr.type = LTTNG_UST_CHAN_METADATA;
        }
 
-       /* Get the right consumer socket for the application. */
-       socket = consumer_find_socket_by_bitness(app->bits_per_long, consumer);
-       if (!socket) {
-               ret = -EINVAL;
-               goto error_consumer;
-       }
-
        /* Need one fd for the channel. */
        ret = lttng_fd_get(LTTNG_FD_APPS, 1);
        if (ret < 0) {
@@ -2513,6 +2586,13 @@ static int create_ust_app_metadata(struct ust_app_session *ua_sess,
                goto error;
        }
 
+       /* Get the right consumer socket for the application. */
+       socket = consumer_find_socket_by_bitness(app->bits_per_long, consumer);
+       if (!socket) {
+               ret = -EINVAL;
+               goto error_consumer;
+       }
+
        /*
         * Keep metadata key so we can identify it on the consumer side. Assign it
         * to the registry *before* we ask the consumer so we avoid the race of the
@@ -2534,7 +2614,6 @@ static int create_ust_app_metadata(struct ust_app_session *ua_sess,
                 * Safe because the metadata obj pointer is not set so the delete below
                 * will not put a FD back again.
                 */
-               lttng_fd_put(LTTNG_FD_APPS, 1);
                goto error_consumer;
        }
 
@@ -2550,7 +2629,6 @@ static int create_ust_app_metadata(struct ust_app_session *ua_sess,
                 * Safe because the metadata obj pointer is not set so the delete below
                 * will not put a FD back again.
                 */
-               lttng_fd_put(LTTNG_FD_APPS, 1);
                goto error_consumer;
        }
 
@@ -2558,6 +2636,7 @@ static int create_ust_app_metadata(struct ust_app_session *ua_sess,
                        metadata->key, app->pid);
 
 error_consumer:
+       lttng_fd_put(LTTNG_FD_APPS, 1);
        delete_ust_app_channel(-1, metadata, app);
 error:
        return ret;
@@ -2805,15 +2884,18 @@ void ust_app_unregister(int sock)
                 * session so the delete session will NOT push/close a second time.
                 */
                registry = get_session_registry(ua_sess);
-               if (registry) {
+               if (registry && !registry->metadata_closed) {
                        /* Push metadata for application before freeing the application. */
                        (void) push_metadata(registry, ua_sess->consumer);
 
                        /*
                         * Don't ask to close metadata for global per UID buffers. Close
-                        * metadata only on destroy trace session in this case.
+                        * metadata only on destroy trace session in this case. Also, the
+                        * previous push metadata could have flag the metadata registry to
+                        * close so don't send a close command if closed.
                         */
-                       if (ua_sess->buffer_type != LTTNG_BUFFER_PER_UID) {
+                       if (ua_sess->buffer_type != LTTNG_BUFFER_PER_UID &&
+                                       !registry->metadata_closed) {
                                /* And ask to close it for this session registry. */
                                (void) close_metadata(registry, ua_sess->consumer);
                        }
@@ -3039,6 +3121,8 @@ error:
 
 /*
  * Free and clean all traceable apps of the global list.
+ *
+ * Should _NOT_ be called with RCU read-side lock held.
  */
 void ust_app_clean_list(void)
 {
@@ -3069,13 +3153,12 @@ void ust_app_clean_list(void)
                ret = lttng_ht_del(ust_app_ht_by_notify_sock, &iter);
                assert(!ret);
        }
+       rcu_read_unlock();
 
        /* Destroy is done only when the ht is empty */
-       lttng_ht_destroy(ust_app_ht);
-       lttng_ht_destroy(ust_app_ht_by_sock);
-       lttng_ht_destroy(ust_app_ht_by_notify_sock);
-
-       rcu_read_unlock();
+       ht_cleanup_push(ust_app_ht);
+       ht_cleanup_push(ust_app_ht_by_sock);
+       ht_cleanup_push(ust_app_ht_by_notify_sock);
 }
 
 /*
@@ -3543,6 +3626,7 @@ int ust_app_create_event_glb(struct ltt_ust_session *usess,
 /*
  * Start tracing for a specific UST session and app.
  */
+static
 int ust_app_start_trace(struct ltt_ust_session *usess, struct ust_app *app)
 {
        int ret = 0;
@@ -3635,12 +3719,11 @@ error_unlock:
 /*
  * Stop tracing for a specific UST session and app.
  */
+static
 int ust_app_stop_trace(struct ltt_ust_session *usess, struct ust_app *app)
 {
        int ret = 0;
-       struct lttng_ht_iter iter;
        struct ust_app_session *ua_sess;
-       struct ust_app_channel *ua_chan;
        struct ust_registry_session *registry;
 
        DBG("Stopping tracing for ust app pid %d", app->pid);
@@ -3693,6 +3776,55 @@ int ust_app_stop_trace(struct ltt_ust_session *usess, struct ust_app *app)
 
        health_code_update();
 
+       registry = get_session_registry(ua_sess);
+       assert(registry);
+
+       if (!registry->metadata_closed) {
+               /* Push metadata for application before freeing the application. */
+               (void) push_metadata(registry, ua_sess->consumer);
+       }
+
+       pthread_mutex_unlock(&ua_sess->lock);
+end_no_session:
+       rcu_read_unlock();
+       health_code_update();
+       return 0;
+
+error_rcu_unlock:
+       pthread_mutex_unlock(&ua_sess->lock);
+       rcu_read_unlock();
+       health_code_update();
+       return -1;
+}
+
+/*
+ * Flush buffers for a specific UST session and app.
+ */
+static
+int ust_app_flush_trace(struct ltt_ust_session *usess, struct ust_app *app)
+{
+       int ret = 0;
+       struct lttng_ht_iter iter;
+       struct ust_app_session *ua_sess;
+       struct ust_app_channel *ua_chan;
+
+       DBG("Flushing buffers for ust app pid %d", app->pid);
+
+       rcu_read_lock();
+
+       if (!app->compatible) {
+               goto end_no_session;
+       }
+
+       ua_sess = lookup_session_by_app(usess, app);
+       if (ua_sess == NULL) {
+               goto end_no_session;
+       }
+
+       pthread_mutex_lock(&ua_sess->lock);
+
+       health_code_update();
+
        /* Flushing buffers */
        cds_lfht_for_each_entry(ua_sess->channels->ht, &iter.iter, ua_chan,
                        node.node) {
@@ -3716,22 +3848,11 @@ int ust_app_stop_trace(struct ltt_ust_session *usess, struct ust_app *app)
 
        health_code_update();
 
-       registry = get_session_registry(ua_sess);
-       assert(registry);
-       /* Push metadata for application before freeing the application. */
-       (void) push_metadata(registry, ua_sess->consumer);
-
        pthread_mutex_unlock(&ua_sess->lock);
 end_no_session:
        rcu_read_unlock();
        health_code_update();
        return 0;
-
-error_rcu_unlock:
-       pthread_mutex_unlock(&ua_sess->lock);
-       rcu_read_unlock();
-       health_code_update();
-       return -1;
 }
 
 /*
@@ -3816,9 +3937,21 @@ int ust_app_stop_trace_all(struct ltt_ust_session *usess)
 
        rcu_read_lock();
 
-       /* Flush all per UID buffers associated to that session. */
-       if (usess->buffer_type == LTTNG_BUFFER_PER_UID) {
+       cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, pid_n.node) {
+               ret = ust_app_stop_trace(usess, app);
+               if (ret < 0) {
+                       /* Continue to next apps even on error */
+                       continue;
+               }
+       }
+
+       /* Flush buffers */
+       switch (usess->buffer_type) {
+       case LTTNG_BUFFER_PER_UID:
+       {
                struct buffer_reg_uid *reg;
+
+               /* Flush all per UID buffers associated to that session. */
                cds_list_for_each_entry(reg, &usess->buffer_reg_uid_list, lnode) {
                        struct buffer_reg_channel *reg_chan;
                        struct consumer_socket *socket;
@@ -3841,14 +3974,20 @@ int ust_app_stop_trace_all(struct ltt_ust_session *usess)
                                (void) consumer_flush_channel(socket, reg_chan->consumer_key);
                        }
                }
+               break;
        }
-
-       cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, pid_n.node) {
-               ret = ust_app_stop_trace(usess, app);
-               if (ret < 0) {
-                       /* Continue to next apps even on error */
-                       continue;
+       case LTTNG_BUFFER_PER_PID:
+               cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, pid_n.node) {
+                       ret = ust_app_flush_trace(usess, app);
+                       if (ret < 0) {
+                               /* Continue to next apps even on error */
+                               continue;
+                       }
                }
+               break;
+       default:
+               assert(0);
+               break;
        }
 
        rcu_read_unlock();
@@ -4337,9 +4476,14 @@ static int reply_ust_register_channel(int sock, int sobjd, int cobjd,
                goto error_rcu_unlock;
        }
 
-       /* Lookup channel by UST object descriptor. Should always be found. */
+       /* Lookup channel by UST object descriptor. */
        ua_chan = find_channel_by_objd(app, cobjd);
-       assert(ua_chan);
+       if (!ua_chan) {
+               DBG("Application channel is being teardown. Abort event notify");
+               ret = 0;
+               goto error_rcu_unlock;
+       }
+
        assert(ua_chan->session);
        ua_sess = ua_chan->session;
 
@@ -4443,9 +4587,14 @@ static int add_event_ust_registry(int sock, int sobjd, int cobjd, char *name,
                goto error_rcu_unlock;
        }
 
-       /* Lookup channel by UST object descriptor. Should always be found. */
+       /* Lookup channel by UST object descriptor. */
        ua_chan = find_channel_by_objd(app, cobjd);
-       assert(ua_chan);
+       if (!ua_chan) {
+               DBG("Application channel is being teardown. Abort event notify");
+               ret = 0;
+               goto error_rcu_unlock;
+       }
+
        assert(ua_chan->session);
        ua_sess = ua_chan->session;
 
This page took 0.031543 seconds and 4 git commands to generate.