From: David Goulet Date: Fri, 28 Sep 2012 19:43:19 +0000 (-0400) Subject: Fix: Metadata stream leak when received in consumer X-Git-Tag: v2.1.0-rc5~41 X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=633d0084183d5b0aed953ce267e0e57e5ef29bd5 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 --- diff --git a/src/common/consumer.c b/src/common/consumer.c index a9e4dee66..53806b08d 100644 --- 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 a288df3cd..4d61cc506 100644 --- 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 27cb58e52..cf3649397 100644 --- 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 9e1a59f5e..f57e2e602 100644 --- 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);