Fix: unchecked return value in ust app delete
[lttng-tools.git] / src / bin / lttng-sessiond / ust-app.c
index eea0d8c03aee26a1e8376ba18b94a6f537d632ec..9a8a11acab6292c36c6a2350af7c4be2e22c1c10 100644 (file)
@@ -364,6 +364,7 @@ void delete_ust_app_channel(int sock, struct ust_app_channel *ua_chan,
 
        /* Wipe context */
        cds_lfht_for_each_entry(ua_chan->ctx->ht, &iter.iter, ua_ctx, node.node) {
+               cds_list_del(&ua_ctx->list);
                ret = lttng_ht_del(ua_chan->ctx, &iter);
                assert(!ret);
                delete_ust_app_ctx(sock, ua_ctx);
@@ -377,16 +378,19 @@ void delete_ust_app_channel(int sock, struct ust_app_channel *ua_chan,
                delete_ust_app_event(sock, ua_event);
        }
 
-       /* Wipe and free registry from session registry. */
-       registry = get_session_registry(ua_chan->session);
-       if (registry) {
-               ust_registry_channel_del_free(registry, ua_chan->key);
+       if (ua_chan->session->buffer_type == LTTNG_BUFFER_PER_PID) {
+               /* Wipe and free registry from session registry. */
+               registry = get_session_registry(ua_chan->session);
+               if (registry) {
+                       ust_registry_channel_del_free(registry, ua_chan->key);
+               }
        }
 
        if (ua_chan->obj != NULL) {
                /* Remove channel from application UST object descriptor. */
                iter.iter.node = &ua_chan->ust_objd_node.node;
-               lttng_ht_del(app->ust_objd, &iter);
+               ret = lttng_ht_del(app->ust_objd, &iter);
+               assert(!ret);
                ret = ustctl_release_object(sock, ua_chan->obj);
                if (ret < 0 && ret != -EPIPE && ret != -LTTNG_UST_ERR_EXITING) {
                        ERR("UST app sock %d release channel obj failed with ret %d",
@@ -461,6 +465,25 @@ push_data:
        ret = consumer_push_metadata(socket, registry->metadata_key,
                        metadata_str, len, offset);
        if (ret < 0) {
+               /*
+                * There is an acceptable race here between the registry metadata key
+                * assignment and the creation on the consumer. The session daemon can
+                * concurrently push metadata for this registry while being created on
+                * the consumer since the metadata key of the registry is assigned
+                * *before* it is setup to avoid the consumer to ask for metadata that
+                * could possibly be not found in the session daemon.
+                *
+                * The metadata will get pushed either by the session being stopped or
+                * the consumer requesting metadata if that race is triggered.
+                */
+               if (ret == -LTTCOMM_CONSUMERD_CHANNEL_FAIL) {
+                       ret = 0;
+               }
+
+               /* Update back the actual metadata len sent since it failed here. */
+               pthread_mutex_lock(&registry->lock);
+               registry->metadata_len_sent -= len;
+               pthread_mutex_unlock(&registry->lock);
                ret_val = ret;
                goto error_push;
        }
@@ -823,6 +846,7 @@ struct ust_app_channel *alloc_ust_app_channel(char *name,
        lttng_ht_node_init_str(&ua_chan->node, ua_chan->name);
 
        CDS_INIT_LIST_HEAD(&ua_chan->streams.head);
+       CDS_INIT_LIST_HEAD(&ua_chan->ctx_list);
 
        /* Copy attributes */
        if (attr) {
@@ -914,6 +938,8 @@ struct ust_app_ctx *alloc_ust_app_ctx(struct lttng_ust_context *uctx)
                goto error;
        }
 
+       CDS_INIT_LIST_HEAD(&ua_ctx->list);
+
        if (uctx) {
                memcpy(&ua_ctx->ctx, uctx, sizeof(ua_ctx->ctx));
        }
@@ -1049,6 +1075,12 @@ int create_ust_channel_context(struct ust_app_channel *ua_chan,
                        ERR("UST app create channel context failed for app (pid: %d) "
                                        "with ret %d", app->pid, ret);
                } else {
+                       /*
+                        * This is normal behavior, an application can die during the
+                        * creation process. Don't report an error so the execution can
+                        * continue normally.
+                        */
+                       ret = 0;
                        DBG3("UST app disable event failed. Application is dead.");
                }
                goto error;
@@ -1087,6 +1119,12 @@ int set_ust_event_filter(struct ust_app_event *ua_event,
                        ERR("UST app event %s filter failed for app (pid: %d) "
                                        "with ret %d", ua_event->attr.name, app->pid, ret);
                } else {
+                       /*
+                        * This is normal behavior, an application can die during the
+                        * creation process. Don't report an error so the execution can
+                        * continue normally.
+                        */
+                       ret = 0;
                        DBG3("UST app filter event failed. Application is dead.");
                }
                goto error;
@@ -1116,6 +1154,12 @@ static int disable_ust_event(struct ust_app *app,
                                        "and session handle %d with ret %d",
                                        ua_event->attr.name, app->pid, ua_sess->handle, ret);
                } else {
+                       /*
+                        * This is normal behavior, an application can die during the
+                        * creation process. Don't report an error so the execution can
+                        * continue normally.
+                        */
+                       ret = 0;
                        DBG3("UST app disable event failed. Application is dead.");
                }
                goto error;
@@ -1146,6 +1190,12 @@ static int disable_ust_channel(struct ust_app *app,
                                        "and session handle %d with ret %d",
                                        ua_chan->name, app->pid, ua_sess->handle, ret);
                } else {
+                       /*
+                        * This is normal behavior, an application can die during the
+                        * creation process. Don't report an error so the execution can
+                        * continue normally.
+                        */
+                       ret = 0;
                        DBG3("UST app disable channel failed. Application is dead.");
                }
                goto error;
@@ -1176,6 +1226,12 @@ static int enable_ust_channel(struct ust_app *app,
                                        "and session handle %d with ret %d",
                                        ua_chan->name, app->pid, ua_sess->handle, ret);
                } else {
+                       /*
+                        * This is normal behavior, an application can die during the
+                        * creation process. Don't report an error so the execution can
+                        * continue normally.
+                        */
+                       ret = 0;
                        DBG3("UST app enable channel failed. Application is dead.");
                }
                goto error;
@@ -1208,6 +1264,12 @@ static int enable_ust_event(struct ust_app *app,
                                        "and session handle %d with ret %d",
                                        ua_event->attr.name, app->pid, ua_sess->handle, ret);
                } else {
+                       /*
+                        * This is normal behavior, an application can die during the
+                        * creation process. Don't report an error so the execution can
+                        * continue normally.
+                        */
+                       ret = 0;
                        DBG3("UST app enable event failed. Application is dead.");
                }
                goto error;
@@ -1288,6 +1350,12 @@ int create_ust_event(struct ust_app *app, struct ust_app_session *ua_sess,
                        ERR("Error ustctl create event %s for app pid: %d with ret %d",
                                        ua_event->attr.name, app->pid, ret);
                } else {
+                       /*
+                        * This is normal behavior, an application can die during the
+                        * creation process. Don't report an error so the execution can
+                        * continue normally.
+                        */
+                       ret = 0;
                        DBG3("UST app create event failed. Application is dead.");
                }
                goto error;
@@ -1393,7 +1461,7 @@ static void shadow_copy_channel(struct ust_app_channel *ua_chan,
        ua_chan->enabled = uchan->enabled;
        ua_chan->tracing_channel_id = uchan->id;
 
-       cds_lfht_for_each_entry(uchan->ctx->ht, &iter.iter, uctx, node.node) {
+       cds_list_for_each_entry(uctx, &uchan->ctx_list, list) {
                ua_ctx = alloc_ust_app_ctx(&uctx->ctx);
                if (ua_ctx == NULL) {
                        continue;
@@ -1401,6 +1469,7 @@ static void shadow_copy_channel(struct ust_app_channel *ua_chan,
                lttng_ht_node_init_ulong(&ua_ctx->node,
                                (unsigned long) ua_ctx->ctx.ctx);
                lttng_ht_add_unique_ulong(ua_chan->ctx, &ua_ctx->node);
+               cds_list_add_tail(&ua_ctx->list, &ua_chan->ctx_list);
        }
 
        /* Copy all events from ltt ust channel to ust app channel */
@@ -1724,6 +1793,13 @@ static int create_ust_app_session(struct ltt_ust_session *usess,
                                                app->pid, ret);
                        } else {
                                DBG("UST app creating session failed. Application is dead");
+                               /*
+                                * This is normal behavior, an application can die during the
+                                * creation process. Don't report an error so the execution can
+                                * continue normally. This will get flagged ENOTCONN and the
+                                * caller will handle it.
+                                */
+                               ret = 0;
                        }
                        delete_ust_app_session(-1, ua_sess, app);
                        if (ret != -ENOMEM) {
@@ -1792,6 +1868,7 @@ int create_ust_app_channel_context(struct ust_app_session *ua_sess,
 
        lttng_ht_node_init_ulong(&ua_ctx->node, (unsigned long) ua_ctx->ctx.ctx);
        lttng_ht_add_unique_ulong(ua_chan->ctx, &ua_ctx->node);
+       cds_list_add_tail(&ua_ctx->list, &ua_chan->ctx_list);
 
        ret = create_ust_channel_context(ua_chan, ua_ctx, app);
        if (ret < 0) {
@@ -2990,6 +3067,12 @@ int ust_app_list_events(struct lttng_event **events)
                                                        app->sock, ret);
                                } else {
                                        DBG3("UST app tp list get failed. Application is dead");
+                                       /*
+                                        * This is normal behavior, an application can die during the
+                                        * creation process. Don't report an error so the execution can
+                                        * continue normally. Continue normal execution.
+                                        */
+                                       break;
                                }
                                goto rcu_error;
                        }
@@ -3084,6 +3167,12 @@ int ust_app_list_event_fields(struct lttng_event_field **fields)
                                                        app->sock, ret);
                                } else {
                                        DBG3("UST app tp list field failed. Application is dead");
+                                       /*
+                                        * This is normal behavior, an application can die during the
+                                        * creation process. Don't report an error so the execution can
+                                        * continue normally.
+                                        */
+                                       break;
                                }
                                goto rcu_error;
                        }
@@ -3107,12 +3196,13 @@ int ust_app_list_event_fields(struct lttng_event_field **fields)
                        }
 
                        memcpy(tmp_event[count].field_name, uiter.field_name, LTTNG_UST_SYM_NAME_LEN);
-                       tmp_event[count].type = uiter.type;
+                       /* Mapping between these enums matches 1 to 1. */
+                       tmp_event[count].type = (enum lttng_event_field_type) uiter.type;
                        tmp_event[count].nowrite = uiter.nowrite;
 
                        memcpy(tmp_event[count].event.name, uiter.event_name, LTTNG_UST_SYM_NAME_LEN);
                        tmp_event[count].event.loglevel = uiter.loglevel;
-                       tmp_event[count].event.type = LTTNG_UST_TRACEPOINT;
+                       tmp_event[count].event.type = LTTNG_EVENT_TRACEPOINT;
                        tmp_event[count].event.pid = app->pid;
                        tmp_event[count].event.enabled = -1;
                        count++;
@@ -3699,6 +3789,13 @@ skip_setup:
                                        app->pid, ret);
                } else {
                        DBG("UST app start session failed. Application is dead.");
+                       /*
+                        * This is normal behavior, an application can die during the
+                        * creation process. Don't report an error so the execution can
+                        * continue normally.
+                        */
+                       pthread_mutex_unlock(&ua_sess->lock);
+                       goto end;
                }
                goto error_unlock;
        }
@@ -3774,6 +3871,12 @@ int ust_app_stop_trace(struct ltt_ust_session *usess, struct ust_app *app)
                                        app->pid, ret);
                } else {
                        DBG("UST app stop session failed. Application is dead.");
+                       /*
+                        * This is normal behavior, an application can die during the
+                        * creation process. Don't report an error so the execution can
+                        * continue normally.
+                        */
+                       goto end_unlock;
                }
                goto error_rcu_unlock;
        }
@@ -3797,6 +3900,7 @@ int ust_app_stop_trace(struct ltt_ust_session *usess, struct ust_app *app)
                (void) push_metadata(registry, ua_sess->consumer);
        }
 
+end_unlock:
        pthread_mutex_unlock(&ua_sess->lock);
 end_no_session:
        rcu_read_unlock();
@@ -3851,8 +3955,11 @@ int ust_app_flush_trace(struct ltt_ust_session *usess, struct ust_app *app)
                        } else {
                                DBG3("UST app failed to flush %s. Application is dead.",
                                                ua_chan->name);
-                               /* No need to continue. */
-                               break;
+                               /*
+                                * This is normal behavior, an application can die during the
+                                * creation process. Don't report an error so the execution can
+                                * continue normally.
+                                */
                        }
                        /* Continuing flushing all buffers */
                        continue;
@@ -3958,7 +4065,7 @@ int ust_app_stop_trace_all(struct ltt_ust_session *usess)
                }
        }
 
-       /* Flush buffers */
+       /* Flush buffers and push metadata (for UID buffers). */
        switch (usess->buffer_type) {
        case LTTNG_BUFFER_PER_UID:
        {
@@ -3966,6 +4073,7 @@ int ust_app_stop_trace_all(struct ltt_ust_session *usess)
 
                /* Flush all per UID buffers associated to that session. */
                cds_list_for_each_entry(reg, &usess->buffer_reg_uid_list, lnode) {
+                       struct ust_registry_session *ust_session_reg;
                        struct buffer_reg_channel *reg_chan;
                        struct consumer_socket *socket;
 
@@ -3986,7 +4094,14 @@ int ust_app_stop_trace_all(struct ltt_ust_session *usess)
                                 */
                                (void) consumer_flush_channel(socket, reg_chan->consumer_key);
                        }
+
+                       ust_session_reg = reg->registry->reg.ust;
+                       if (!ust_session_reg->metadata_closed) {
+                               /* Push metadata. */
+                               (void) push_metadata(ust_session_reg, usess->consumer);
+                       }
                }
+
                break;
        }
        case LTTNG_BUFFER_PER_PID:
@@ -4040,7 +4155,7 @@ int ust_app_destroy_trace_all(struct ltt_ust_session *usess)
 void ust_app_global_update(struct ltt_ust_session *usess, int sock)
 {
        int ret = 0;
-       struct lttng_ht_iter iter, uiter, iter_ctx;
+       struct lttng_ht_iter iter, uiter;
        struct ust_app *app;
        struct ust_app_session *ua_sess = NULL;
        struct ust_app_channel *ua_chan;
@@ -4112,8 +4227,11 @@ void ust_app_global_update(struct ltt_ust_session *usess, int sock)
                        }
                }
 
-               cds_lfht_for_each_entry(ua_chan->ctx->ht, &iter_ctx.iter, ua_ctx,
-                               node.node) {
+               /*
+                * Add context using the list so they are enabled in the same order the
+                * user added them.
+                */
+               cds_list_for_each_entry(ua_ctx, &ua_chan->ctx_list, list) {
                        ret = create_ust_channel_context(ua_chan, ua_ctx, app);
                        if (ret < 0) {
                                goto error_unlock;
@@ -4643,7 +4761,8 @@ static int add_event_ust_registry(int sock, int sobjd, int cobjd, char *name,
         */
        ret_code = ust_registry_create_event(registry, chan_reg_key,
                        sobjd, cobjd, name, sig, nr_fields, fields, loglevel,
-                       model_emf_uri, ua_sess->buffer_type, &event_id);
+                       model_emf_uri, ua_sess->buffer_type, &event_id,
+                       app);
 
        /*
         * The return value is returned to ustctl so in case of an error, the
This page took 0.028478 seconds and 4 git commands to generate.