Fix: Metadata stream leak when received in consumer

Between threads, when the metadata stream is received, it is allocated.
We now pass the pointer to the metadata thread thus fixing a memory leak
because the original stream was never freed.

This commit also modified some debug statements and remove a duplicate
code snippet.

Signed-off-by: David Goulet <dgoulet@efficios.com>

diff --git a/src/common/consumer.c b/src/common/consumer.c
index a9e4dee661b1d1ba13da01a2db4d74ee16f68e8e..53806b08d8ee4a0dcd54b80ee71edaca221fd784 100644 (file)
--- a/src/common/consumer.c
+++ b/src/common/consumer.c
@@ -1723,20 +1723,13 @@ restart:
                                        close(ctx->consumer_metadata_pipe[0]);
                                        continue;
                                } else if (revents & LPOLLIN) {
-                                       stream = zmalloc(sizeof(struct lttng_consumer_stream));
-                                       if (stream == NULL) {
-                                               PERROR("zmalloc metadata consumer stream");
-                                               goto error;
-                                       }
-
                                        do {
-                                               /* Get the stream and add it to the local hash table */
-                                               ret = read(pollfd, stream,
-                                                               sizeof(struct lttng_consumer_stream));
+                                               /* Get the stream pointer received */
+                                               ret = read(pollfd, &stream, sizeof(stream));
                                        } while (ret < 0 && errno == EINTR);
-                                       if (ret < 0 || ret < sizeof(struct lttng_consumer_stream)) {
+                                       if (ret < 0 ||
+                                                       ret < sizeof(struct lttng_consumer_stream *)) {
                                                PERROR("read metadata stream");
-                                               free(stream);
                                                /*
                                                 * Let's continue here and hope we can still work
                                                 * without stopping the consumer. XXX: Should we?

diff --git a/src/common/kernel-consumer/kernel-consumer.c b/src/common/kernel-consumer/kernel-consumer.c
index a288df3cd5f9f3e0fc79b793ff7720f229ee1f61..4d61cc506f32e57896b96bf72ee50b91cc3a50b5 100644 (file)
--- a/src/common/kernel-consumer/kernel-consumer.c
+++ b/src/common/kernel-consumer/kernel-consumer.c
@@ -215,29 +215,23 @@ int lttng_kconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                        goto end_nosignal;
                }
 
-               /* Send stream to the metadata thread */
-               if (new_stream->metadata_flag) {
-                       if (ctx->on_recv_stream) {
-                               ret = ctx->on_recv_stream(new_stream);
-                               if (ret < 0) {
-                                       goto end_nosignal;
-                               }
+               if (ctx->on_recv_stream) {
+                       ret = ctx->on_recv_stream(new_stream);
+                       if (ret < 0) {
+                               goto end_nosignal;
                        }
+               }
 
+               /* Send stream to the metadata thread */
+               if (new_stream->metadata_flag) {
                        do {
-                               ret = write(ctx->consumer_metadata_pipe[1], new_stream,
-                                               sizeof(struct lttng_consumer_stream));
+                               ret = write(ctx->consumer_metadata_pipe[1], &new_stream,
+                                               sizeof(new_stream));
                        } while (ret < 0 && errno == EINTR);
                        if (ret < 0) {
                                PERROR("write metadata pipe");
                        }
                } else {
-                       if (ctx->on_recv_stream) {
-                               ret = ctx->on_recv_stream(new_stream);
-                               if (ret < 0) {
-                                       goto end_nosignal;
-                               }
-                       }
                        consumer_add_stream(new_stream);
                }
 

diff --git a/src/common/relayd/relayd.c b/src/common/relayd/relayd.c
index 27cb58e52ae49113f877c9e31b604d349000ffaf..cf3649397425840b9c0c0e0ae07d22073255f0fa 100644 (file)
--- a/src/common/relayd/relayd.c
+++ b/src/common/relayd/relayd.c
@@ -70,7 +70,7 @@ static int send_command(struct lttcomm_sock *sock,
                goto error;
        }
 
-       DBG3("Relayd sending command %d", cmd);
+       DBG3("Relayd sending command %d of size %" PRIu64, cmd, buf_size);
 
 error:
        free(buf);
@@ -86,7 +86,7 @@ static int recv_reply(struct lttcomm_sock *sock, void *data, size_t size)
 {
        int ret;
 
-       DBG3("Relayd waiting for reply...");
+       DBG3("Relayd waiting for reply of size %ld", size);
 
        ret = sock->ops->recvmsg(sock, data, size, 0);
        if (ret < 0) {
@@ -125,7 +125,7 @@ int relayd_add_stream(struct lttcomm_sock *sock, const char *channel_name,
                goto error;
        }
 
-       /* Recevie response */
+       /* Waiting for reply */
        ret = recv_reply(sock, (void *) &reply, sizeof(reply));
        if (ret < 0) {
                goto error;
@@ -228,7 +228,7 @@ int relayd_send_metadata(struct lttcomm_sock *sock, size_t len)
        /*
         * After that call, the metadata data MUST be sent to the relayd so the
         * receive size on the other end matches the len of the metadata packet
-        * header.
+        * header. This is why we don't wait for a reply here.
         */
 
 error:
@@ -273,7 +273,7 @@ int relayd_send_data_hdr(struct lttcomm_sock *sock,
        assert(sock);
        assert(hdr);
 
-       DBG3("Relayd sending data header...");
+       DBG3("Relayd sending data header of size %ld", size);
 
        /* Again, safety net */
        if (size == 0) {

diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c
index 9e1a59f5e447558a127d5ceac8273417131dd59e..f57e2e602f1d6bcf7b2f017fbdc0c933ae7830a8 100644 (file)
--- a/src/common/ust-consumer/ust-consumer.c
+++ b/src/common/ust-consumer/ust-consumer.c
@@ -244,29 +244,24 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
                        goto end_nosignal;
                }
 
-               /* Send stream to the metadata thread */
-               if (new_stream->metadata_flag) {
-                       if (ctx->on_recv_stream) {
-                               ret = ctx->on_recv_stream(new_stream);
-                               if (ret < 0) {
-                                       goto end_nosignal;
-                               }
+               /* Do actions once stream has been received. */
+               if (ctx->on_recv_stream) {
+                       ret = ctx->on_recv_stream(new_stream);
+                       if (ret < 0) {
+                               goto end_nosignal;
                        }
+               }
 
+               /* Send stream to the metadata thread */
+               if (new_stream->metadata_flag) {
                        do {
-                               ret = write(ctx->consumer_metadata_pipe[1], new_stream,
-                                               sizeof(struct lttng_consumer_stream));
+                               ret = write(ctx->consumer_metadata_pipe[1], &new_stream,
+                                               sizeof(new_stream));
                        } while (ret < 0 && errno == EINTR);
                        if (ret < 0) {
                                PERROR("write metadata pipe");
                        }
                } else {
-                       if (ctx->on_recv_stream) {
-                               ret = ctx->on_recv_stream(new_stream);
-                               if (ret < 0) {
-                                       goto end_nosignal;
-                               }
-                       }
                        consumer_add_stream(new_stream);
                }
 
@@ -481,7 +476,6 @@ int lttng_ustconsumer_read_subbuffer(struct lttng_consumer_stream *stream,
                ERR("Error writing to tracefile "
                                "(ret: %zd != len: %lu != subbuf_size: %lu)",
                                ret, len, subbuf_size);
-
        }
        err = ustctl_put_next_subbuf(handle, buf);
        assert(err == 0);