Fix: wrong check before destroying the viewer metadata stream
[lttng-tools.git] / src / bin / lttng-relayd / live.c
index e51ff5cdf53495d4848a2867cfc952e9f2534bc5..53cc660918d463aacae382c6a2c858281925f26d 100644 (file)
@@ -478,7 +478,7 @@ int viewer_connect(struct relay_command *cmd)
        if (reply.major != be32toh(msg.major)) {
                DBG("Incompatible major versions (%u vs %u)", reply.major,
                                be32toh(msg.major));
-               ret = 0;
+               ret = -1;
                goto end;
        }
 
@@ -614,7 +614,7 @@ static int open_index(struct relay_viewer_stream *stream)
 {
        int ret;
        char fullpath[PATH_MAX];
-       struct lttng_packet_index_file_hdr hdr;
+       struct ctf_packet_index_file_hdr hdr;
 
        if (stream->tracefile_count > 0) {
                ret = snprintf(fullpath, sizeof(fullpath), "%s/" DEFAULT_INDEX_DIR "/%s_%"
@@ -649,13 +649,13 @@ static int open_index(struct relay_viewer_stream *stream)
                PERROR("Reading index header");
                goto error;
        }
-       if (strncmp(hdr.magic, INDEX_MAGIC, sizeof(hdr.magic)) != 0) {
+       if (be32toh(hdr.magic) != CTF_INDEX_MAGIC) {
                ERR("Invalid header magic");
                ret = -1;
                goto error;
        }
-       if (be32toh(hdr.index_major) != INDEX_MAJOR ||
-                       be32toh(hdr.index_minor) != INDEX_MINOR) {
+       if (be32toh(hdr.index_major) != CTF_INDEX_MAJOR ||
+                       be32toh(hdr.index_minor) != CTF_INDEX_MINOR) {
                ERR("Invalid header version");
                ret = -1;
                goto error;
@@ -698,6 +698,7 @@ int init_viewer_stream(struct relay_stream *stream, int seek_last)
                        LTTNG_VIEWER_NAME_MAX);
        viewer_stream->tracefile_count = stream->tracefile_count;
        viewer_stream->metadata_flag = stream->metadata_flag;
+       viewer_stream->tracefile_count_last = -1ULL;
        if (seek_last) {
                viewer_stream->tracefile_count_current =
                        stream->tracefile_count_current;
@@ -706,11 +707,11 @@ int init_viewer_stream(struct relay_stream *stream, int seek_last)
                        stream->oldest_tracefile_id;
        }
 
-       /*
-        * The deletion of this ctf_trace object is only done in a call RCU of the
-        * relay stream making it valid as long as we have the read side lock.
-        */
        viewer_stream->ctf_trace = stream->ctf_trace;
+       if (viewer_stream->metadata_flag) {
+               viewer_stream->ctf_trace->viewer_metadata_stream =
+                       viewer_stream;
+       }
        uatomic_inc(&viewer_stream->ctf_trace->refcount);
 
        lttng_ht_node_init_u64(&viewer_stream->stream_n, stream->stream_handle);
@@ -746,7 +747,7 @@ int init_viewer_stream(struct relay_stream *stream, int seek_last)
        if (seek_last && viewer_stream->index_read_fd > 0) {
                ret = lseek(viewer_stream->index_read_fd,
                                viewer_stream->total_index_received *
-                                       sizeof(struct lttng_packet_index),
+                                       sizeof(struct ctf_packet_index),
                                SEEK_CUR);
                if (ret < 0) {
                        goto error;
@@ -764,7 +765,7 @@ error:
 /*
  * Rotate a stream to the next tracefile.
  *
- * Returns 0 on success, a negative value on error.
+ * Returns 0 on success, 1 on EOF, a negative value on error.
  */
 static
 int rotate_viewer_stream(struct relay_viewer_stream *viewer_stream,
@@ -777,6 +778,15 @@ int rotate_viewer_stream(struct relay_viewer_stream *viewer_stream,
 
        tracefile_id = (viewer_stream->tracefile_count_current + 1) %
                viewer_stream->tracefile_count;
+       /*
+        * Detect the last tracefile to open.
+        */
+       if (viewer_stream->tracefile_count_last != -1ULL &&
+                       viewer_stream->tracefile_count_last ==
+                       viewer_stream->tracefile_count_current) {
+               ret = 1;
+               goto end;
+       }
 
        if (stream) {
                pthread_mutex_lock(&stream->viewer_stream_rotation_lock);
@@ -801,23 +811,30 @@ int rotate_viewer_stream(struct relay_viewer_stream *viewer_stream,
                }
        }
        viewer_stream->tracefile_count_current = tracefile_id;
-       pthread_mutex_unlock(&stream->viewer_stream_rotation_lock);
 
-       if (viewer_stream->abort_flag == 0) {
-               ret = close(viewer_stream->index_read_fd);
-               if (ret < 0) {
-                       PERROR("close index file");
-               }
-               ret = close(viewer_stream->read_fd);
-               if (ret < 0) {
-                       PERROR("close tracefile");
-               }
-       } else {
-               viewer_stream->abort_flag = 0;
+       ret = close(viewer_stream->index_read_fd);
+       if (ret < 0) {
+               PERROR("close index file %d",
+                               viewer_stream->index_read_fd);
        }
+       viewer_stream->index_read_fd = -1;
+       ret = close(viewer_stream->read_fd);
+       if (ret < 0) {
+               PERROR("close tracefile %d",
+                               viewer_stream->read_fd);
+       }
+       viewer_stream->read_fd = -1;
 
+       pthread_mutex_lock(&viewer_stream->overwrite_lock);
+       viewer_stream->abort_flag = 0;
+       pthread_mutex_unlock(&viewer_stream->overwrite_lock);
+
+       viewer_stream->index_read_fd = -1;
        viewer_stream->read_fd = -1;
 
+       if (stream) {
+               pthread_mutex_unlock(&stream->viewer_stream_rotation_lock);
+       }
        ret = open_index(viewer_stream);
        if (ret < 0) {
                goto error;
@@ -825,6 +842,7 @@ int rotate_viewer_stream(struct relay_viewer_stream *viewer_stream,
 
        ret = 0;
 
+end:
 error:
        return ret;
 }
@@ -836,7 +854,8 @@ static
 int viewer_attach_session(struct relay_command *cmd,
                struct lttng_ht *sessions_ht)
 {
-       int ret, send_streams = 0, nb_streams = 0;
+       int ret, send_streams = 0;
+       uint32_t nb_streams = 0, nb_streams_ready = 0;
        struct lttng_viewer_attach_session_request request;
        struct lttng_viewer_attach_session_response response;
        struct lttng_viewer_stream send_stream;
@@ -943,14 +962,16 @@ int viewer_attach_session(struct relay_command *cmd,
                        if (stream->session != cmd->session) {
                                continue;
                        }
+                       nb_streams++;
 
                        /*
-                        * Don't send streams with no ctf_trace, they are not ready to be
-                        * read.
+                        * Don't send streams with no ctf_trace, they are not
+                        * ready to be read.
                         */
-                       if (!stream->ctf_trace) {
+                       if (!stream->ctf_trace || !stream->viewer_ready) {
                                continue;
                        }
+                       nb_streams_ready++;
 
                        vstream = live_find_viewer_stream_by_id(stream->stream_handle);
                        if (!vstream) {
@@ -959,7 +980,11 @@ int viewer_attach_session(struct relay_command *cmd,
                                        goto end_unlock;
                                }
                        }
-                       nb_streams++;
+               }
+
+               /* We must have the same amount of existing stream and ready stream. */
+               if (nb_streams != nb_streams_ready) {
+                       nb_streams = 0;
                }
                response.streams_count = htobe32(nb_streams);
        }
@@ -974,10 +999,10 @@ send_reply:
        health_code_update();
 
        /*
-        * Unknown or busy session, just return gracefully, the viewer knows what
+        * Unknown or empty session, just return gracefully, the viewer knows what
         * is happening.
         */
-       if (!send_streams) {
+       if (!send_streams || !nb_streams) {
                ret = 0;
                goto end_unlock;
        }
@@ -1045,6 +1070,72 @@ end:
        return stream;
 }
 
+static
+void deferred_free_viewer_stream(struct rcu_head *head)
+{
+       struct relay_viewer_stream *stream =
+               caa_container_of(head, struct relay_viewer_stream, rcu_node);
+
+       free(stream->path_name);
+       free(stream->channel_name);
+       free(stream);
+}
+
+static
+void delete_viewer_stream(struct relay_viewer_stream *vstream)
+{
+       int delret;
+       struct lttng_ht_iter iter;
+
+       iter.iter.node = &vstream->stream_n.node;
+       delret = lttng_ht_del(viewer_streams_ht, &iter);
+       assert(!delret);
+}
+
+static
+void destroy_viewer_stream(struct relay_viewer_stream *vstream)
+{
+       unsigned long ret_ref;
+       int ret;
+
+       assert(vstream);
+       ret_ref = uatomic_add_return(&vstream->ctf_trace->refcount, -1);
+       assert(ret_ref >= 0);
+
+       if (vstream->read_fd >= 0) {
+               ret = close(vstream->read_fd);
+               if (ret < 0) {
+                       PERROR("close read_fd");
+               }
+       }
+       if (vstream->index_read_fd >= 0) {
+               ret = close(vstream->index_read_fd);
+               if (ret < 0) {
+                       PERROR("close index_read_fd");
+               }
+       }
+
+       /*
+        * If the only stream left in the HT is the metadata stream,
+        * we need to remove it because we won't detect a EOF for this
+        * stream.
+        */
+       if (ret_ref == 1 && vstream->ctf_trace->viewer_metadata_stream) {
+               destroy_viewer_stream(vstream->ctf_trace->viewer_metadata_stream);
+               vstream->ctf_trace->metadata_stream = NULL;
+               DBG("Freeing ctf_trace %" PRIu64, vstream->ctf_trace->id);
+               /*
+                * The streaming-side is already closed and we can't receive a new
+                * stream concurrently at this point (since the session is being
+                * destroyed), so when we detect the refcount equals 0, we are the
+                * only owners of the ctf_trace and we can free it ourself.
+                */
+               free(vstream->ctf_trace);
+       }
+
+       call_rcu(&vstream->rcu_node, deferred_free_viewer_stream);
+}
+
 /*
  * Send the next index for a stream.
  *
@@ -1057,7 +1148,7 @@ int viewer_get_next_index(struct relay_command *cmd,
        int ret;
        struct lttng_viewer_get_next_index request_index;
        struct lttng_viewer_index viewer_index;
-       struct lttng_packet_index packet_index;
+       struct ctf_packet_index packet_index;
        struct relay_viewer_stream *vstream;
        struct relay_stream *rstream;
 
@@ -1123,33 +1214,41 @@ int viewer_get_next_index(struct relay_command *cmd,
                        ret = rotate_viewer_stream(vstream, rstream);
                        if (ret < 0) {
                                goto end_unlock;
+                       } else if (ret == 1) {
+                               viewer_index.status = htobe32(VIEWER_INDEX_HUP);
+                               delete_viewer_stream(vstream);
+                               destroy_viewer_stream(vstream);
+                               goto send_reply;
                        }
                }
-               if (rstream->beacon_ts_end != -1ULL &&
-                               vstream->last_sent_index == rstream->total_index_received) {
-                       viewer_index.status = htobe32(VIEWER_INDEX_INACTIVE);
-                       viewer_index.timestamp_end = htobe64(rstream->beacon_ts_end);
-                       goto send_reply;
-               }
-               /*
-                * Reader and writer are working in the same tracefile, so we care
-                * about the number of index received and sent. Otherwise, we read
-                * up to EOF.
-                */
                pthread_mutex_lock(&rstream->viewer_stream_rotation_lock);
-               if (rstream->tracefile_count_current == vstream->tracefile_count_current
-                               && rstream->total_index_received <= vstream->last_sent_index
-                               && !vstream->close_write_flag) {
-                       pthread_mutex_unlock(&rstream->viewer_stream_rotation_lock);
-                       /* No new index to send, retry later. */
-                       viewer_index.status = htobe32(VIEWER_INDEX_RETRY);
-                       goto send_reply;
+               if (rstream->tracefile_count_current == vstream->tracefile_count_current) {
+                       if (rstream->beacon_ts_end != -1ULL &&
+                               vstream->last_sent_index == rstream->total_index_received) {
+                               viewer_index.status = htobe32(VIEWER_INDEX_INACTIVE);
+                               viewer_index.timestamp_end = htobe64(rstream->beacon_ts_end);
+                               pthread_mutex_unlock(&rstream->viewer_stream_rotation_lock);
+                               goto send_reply;
+                       /*
+                        * Reader and writer are working in the same tracefile, so we care
+                        * about the number of index received and sent. Otherwise, we read
+                        * up to EOF.
+                        */
+                       } else if (rstream->total_index_received <= vstream->last_sent_index
+                                       && !vstream->close_write_flag) {
+                               pthread_mutex_unlock(&rstream->viewer_stream_rotation_lock);
+                               /* No new index to send, retry later. */
+                               viewer_index.status = htobe32(VIEWER_INDEX_RETRY);
+                               goto send_reply;
+                       }
                }
                pthread_mutex_unlock(&rstream->viewer_stream_rotation_lock);
        } else if (!rstream && vstream->close_write_flag &&
                        vstream->total_index_received == vstream->last_sent_index) {
                /* Last index sent and current tracefile closed in write */
                viewer_index.status = htobe32(VIEWER_INDEX_HUP);
+               delete_viewer_stream(vstream);
+               destroy_viewer_stream(vstream);
                goto send_reply;
        } else {
                vstream->close_write_flag = 1;
@@ -1161,8 +1260,28 @@ int viewer_get_next_index(struct relay_command *cmd,
                viewer_index.flags |= LTTNG_VIEWER_FLAG_NEW_METADATA;
        }
 
+       pthread_mutex_lock(&vstream->overwrite_lock);
+       if (vstream->abort_flag) {
+               /*
+                * The file is being overwritten by the writer, we cannot
+                * use it.
+                */
+               viewer_index.status = htobe32(VIEWER_INDEX_RETRY);
+               pthread_mutex_unlock(&vstream->overwrite_lock);
+               ret = rotate_viewer_stream(vstream, rstream);
+               if (ret < 0) {
+                       goto end_unlock;
+               } else if (ret == 1) {
+                       viewer_index.status = htobe32(VIEWER_INDEX_HUP);
+                       delete_viewer_stream(vstream);
+                       destroy_viewer_stream(vstream);
+                       goto send_reply;
+               }
+               goto send_reply;
+       }
        ret = lttng_read(vstream->index_read_fd, &packet_index,
                        sizeof(packet_index));
+       pthread_mutex_unlock(&vstream->overwrite_lock);
        if (ret < sizeof(packet_index)) {
                /*
                 * The tracefile is closed in write, so we read up to EOF.
@@ -1173,19 +1292,16 @@ int viewer_get_next_index(struct relay_command *cmd,
                        ret = rotate_viewer_stream(vstream, rstream);
                        if (ret < 0) {
                                goto end_unlock;
-                       }
-               } else {
-                       /*
-                        * If the read fd was closed by the streaming side, the
-                        * abort_flag will be set to 1, otherwise it is an error.
-                        */
-                       if (vstream->abort_flag != 1) {
-                               PERROR("Relay reading index file");
-                               viewer_index.status = htobe32(VIEWER_INDEX_ERR);
-                               goto send_reply;
-                       } else {
+                       } else if (ret == 1) {
                                viewer_index.status = htobe32(VIEWER_INDEX_HUP);
+                               delete_viewer_stream(vstream);
+                               destroy_viewer_stream(vstream);
+                               goto send_reply;
                        }
+               } else {
+                       PERROR("Relay reading index file %d",
+                                       vstream->index_read_fd);
+                       viewer_index.status = htobe32(VIEWER_INDEX_ERR);
                }
                goto send_reply;
        } else {
@@ -1629,66 +1745,38 @@ void deferred_free_connection(struct rcu_head *head)
        free(relay_connection);
 }
 
-static
-void deferred_free_viewer_stream(struct rcu_head *head)
-{
-       struct relay_viewer_stream *stream =
-               caa_container_of(head, struct relay_viewer_stream, rcu_node);
-
-       if (stream->ctf_trace) {
-               uatomic_dec(&stream->ctf_trace->refcount);
-               assert(uatomic_read(&stream->ctf_trace->refcount) >= 0);
-               if (uatomic_read(&stream->ctf_trace->refcount) == 0) {
-                       DBG("Freeing ctf_trace %" PRIu64, stream->ctf_trace->id);
-                       free(stream->ctf_trace);
-               }
-       }
-
-       free(stream->path_name);
-       free(stream->channel_name);
-       free(stream);
-}
-
+/*
+ * Delete all streams for a specific session ID.
+ */
 static
 void viewer_del_streams(uint64_t session_id)
 {
-       int ret;
        struct relay_viewer_stream *stream;
-       struct lttng_ht_node_u64 *node;
        struct lttng_ht_iter iter;
 
        rcu_read_lock();
-       cds_lfht_for_each_entry(viewer_streams_ht->ht, &iter.iter, node, node) {
+       cds_lfht_for_each_entry(viewer_streams_ht->ht, &iter.iter, stream,
+                       stream_n.node) {
                health_code_update();
 
-               node = lttng_ht_iter_get_node_u64(&iter);
-               if (!node) {
-                       continue;
-               }
-
-               stream = caa_container_of(node, struct relay_viewer_stream, stream_n);
                if (stream->session_id != session_id) {
                        continue;
                }
 
-               if (stream->read_fd >= 0) {
-                       ret = close(stream->read_fd);
-                       if (ret < 0) {
-                               PERROR("close read_fd");
-                       }
-               }
-               if (stream->index_read_fd >= 0) {
-                       ret = close(stream->index_read_fd);
-                       if (ret < 0) {
-                               PERROR("close index_read_fd");
-                       }
-               }
-               if (stream->metadata_flag && stream->ctf_trace) {
+               delete_viewer_stream(stream);
+               assert(stream->ctf_trace);
+
+               if (stream->metadata_flag) {
+                       /*
+                        * The metadata viewer stream is destroyed once the refcount on the
+                        * ctf trace goes to 0 in the destroy stream function thus there is
+                        * no explicit call to that function here.
+                        */
                        stream->ctf_trace->metadata_sent = 0;
+                       stream->ctf_trace->viewer_metadata_stream = NULL;
+               } else {
+                       destroy_viewer_stream(stream);
                }
-               ret = lttng_ht_del(viewer_streams_ht, &iter);
-               assert(!ret);
-               call_rcu(&stream->rcu_node, deferred_free_viewer_stream);
        }
        rcu_read_unlock();
 }
@@ -1708,6 +1796,9 @@ void del_connection(struct lttng_ht *relay_connections_ht,
        assert(iter);
        assert(relay_connection);
 
+       DBG("Cleaning connection of session ID %" PRIu64,
+                       relay_connection->session_id);
+
        ret = lttng_ht_del(relay_connections_ht, iter);
        assert(!ret);
 
This page took 0.029187 seconds and 4 git commands to generate.