Fix: consumer_allocate_stream error handling

Fix a memory leak and "be nice" when handling stream alloc errors. Upon
CPU hotplug, it is possible that we receive a stream only after all
other streams are finalized, which means it could happen that we discard
that channel, in the unlikely event that we have cpu hotplug
concurrently with destroy.

Moreover, this fix the return path of channel lookup failure: we were
returning an zeroed stream rather than returning an error, which was
certainly not the intended behavior.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: David Goulet <dgoulet@efficios.com>

diff --git a/src/common/consumer.c b/src/common/consumer.c
index f4eaf705f0b811230d0a9bb1f2f74458c7f7e6d0..a9e4dee661b1d1ba13da01a2db4d74ee16f68e8e 100644 (file)
--- a/src/common/consumer.c
+++ b/src/common/consumer.c
@@ -345,7 +345,8 @@ struct lttng_consumer_stream *consumer_allocate_stream(
                uid_t uid,
                gid_t gid,
                int net_index,
-               int metadata_flag)
+               int metadata_flag,
+               int *alloc_ret)
 {
        struct lttng_consumer_stream *stream;
        int ret;
@@ -353,12 +354,13 @@ struct lttng_consumer_stream *consumer_allocate_stream(
        stream = zmalloc(sizeof(*stream));
        if (stream == NULL) {
                perror("malloc struct lttng_consumer_stream");
-               goto end;
+               *alloc_ret = -ENOMEM;
+               return NULL;
        }
        stream->chan = consumer_find_channel(channel_key);
        if (!stream->chan) {
-               perror("Unable to find channel key");
-               goto end;
+               *alloc_ret = -ENOENT;
+               goto error;
        }
        stream->chan->refcount++;
        stream->key = stream_key;
@@ -387,14 +389,14 @@ struct lttng_consumer_stream *consumer_allocate_stream(
                stream->cpu = stream->chan->cpucount++;
                ret = lttng_ustconsumer_allocate_stream(stream);
                if (ret) {
-                       free(stream);
-                       return NULL;
+                       *alloc_ret = -EINVAL;
+                       goto error;
                }
                break;
        default:
                ERR("Unknown consumer_data type");
-               assert(0);
-               goto end;
+               *alloc_ret = -EINVAL;
+               goto error;
        }
 
        /*
@@ -413,9 +415,11 @@ struct lttng_consumer_stream *consumer_allocate_stream(
                        stream->shm_fd, stream->wait_fd,
                        (unsigned long long) stream->mmap_len, stream->out_fd,
                        stream->net_seq_idx);
-
-end:
        return stream;
+
+error:
+       free(stream);
+       return NULL;
 }
 
 /*

diff --git a/src/common/consumer.h b/src/common/consumer.h
index 0f82a10865f18b2960900db6e937b3768618d1d0..9a93c427915bbd6de3aef1bdbc9440320244092a 100644 (file)
--- a/src/common/consumer.h
+++ b/src/common/consumer.h
@@ -338,7 +338,8 @@ extern struct lttng_consumer_stream *consumer_allocate_stream(
                uid_t uid,
                gid_t gid,
                int net_index,
-               int metadata_flag);
+               int metadata_flag,
+               int *alloc_ret);
 extern int consumer_add_stream(struct lttng_consumer_stream *stream);
 extern void consumer_del_stream(struct lttng_consumer_stream *stream);
 extern void consumer_change_stream_state(int stream_key,

diff --git a/src/common/kernel-consumer/kernel-consumer.c b/src/common/kernel-consumer/kernel-consumer.c
index 5a219fc0b6c543d5c0f5270c3febd4ccfb4adb3a..a288df3cd5f9f3e0fc79b793ff7720f229ee1f61 100644 (file)
--- a/src/common/kernel-consumer/kernel-consumer.c
+++ b/src/common/kernel-consumer/kernel-consumer.c
@@ -141,6 +141,7 @@ int lttng_kconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                int fd;
                struct consumer_relayd_sock_pair *relayd = NULL;
                struct lttng_consumer_stream *new_stream;
+               int alloc_ret = 0;
 
                /* block */
                if (lttng_consumer_poll_socket(consumer_sockpoll) < 0) {
@@ -166,9 +167,23 @@ int lttng_kconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                                msg.u.stream.uid,
                                msg.u.stream.gid,
                                msg.u.stream.net_index,
-                               msg.u.stream.metadata_flag);
+                               msg.u.stream.metadata_flag,
+                               &alloc_ret);
                if (new_stream == NULL) {
-                       lttng_consumer_send_error(ctx, LTTCOMM_CONSUMERD_OUTFD_ERROR);
+                       switch (alloc_ret) {
+                       case -ENOMEM:
+                       case -EINVAL:
+                       default:
+                               lttng_consumer_send_error(ctx, LTTCOMM_CONSUMERD_OUTFD_ERROR);
+                               break;
+                       case -ENOENT:
+                               /*
+                                * We could not find the channel. Can happen if cpu hotplug
+                                * happens while tearing down.
+                                */
+                               DBG3("Could not find channel");
+                               break;
+                       }
                        goto end_nosignal;
                }
 

diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c
index ad4b014c6f08dc09f4e47b22e19cbd47330ec5ae..9e1a59f5e447558a127d5ceac8273417131dd59e 100644 (file)
--- a/src/common/ust-consumer/ust-consumer.c
+++ b/src/common/ust-consumer/ust-consumer.c
@@ -174,6 +174,7 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                int fds[2];
                size_t nb_fd = 2;
                struct consumer_relayd_sock_pair *relayd = NULL;
+               int alloc_ret = 0;
 
                DBG("UST Consumer adding stream");
 
@@ -204,9 +205,23 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                                msg.u.stream.uid,
                                msg.u.stream.gid,
                                msg.u.stream.net_index,
-                               msg.u.stream.metadata_flag);
+                               msg.u.stream.metadata_flag,
+                               &alloc_ret);
                if (new_stream == NULL) {
-                       lttng_consumer_send_error(ctx, LTTCOMM_CONSUMERD_OUTFD_ERROR);
+                       switch (alloc_ret) {
+                       case -ENOMEM:
+                       case -EINVAL:
+                       default:
+                               lttng_consumer_send_error(ctx, LTTCOMM_CONSUMERD_OUTFD_ERROR);
+                               break;
+                       case -ENOENT:
+                               /*
+                                * We could not find the channel. Can happen if cpu hotplug
+                                * happens while tearing down.
+                                */
+                               DBG3("Could not find channel");
+                               break;
+                       }
                        goto end_nosignal;
                }