From bb35f032b52330cfb17c144bfed81f2b1569f308 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Tue, 25 Aug 2015 12:35:27 -0400 Subject: [PATCH] Fix: Use list rather than ptr array for trace streams MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This fixes stream removal on HUP. Signed-off-by: Mathieu Desnoyers Signed-off-by: Julien Desfossez Signed-off-by: Jérémie Galarneau --- formats/lttng-live/lttng-live-comm.c | 47 ++++++++++++++++++-------- formats/lttng-live/lttng-live-plugin.c | 8 +++-- formats/lttng-live/lttng-live.h | 8 +++-- 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/formats/lttng-live/lttng-live-comm.c b/formats/lttng-live/lttng-live-comm.c index ec0a0123..0c6228b4 100644 --- a/formats/lttng-live/lttng-live-comm.c +++ b/formats/lttng-live/lttng-live-comm.c @@ -390,7 +390,7 @@ int lttng_live_ctf_trace_assign(struct lttng_live_viewer_stream *stream, if (!trace) { trace = g_new0(struct lttng_live_ctf_trace, 1); trace->ctf_trace_id = ctf_trace_id; - trace->streams = g_ptr_array_new(); + BT_INIT_LIST_HEAD(&trace->stream_list); g_hash_table_insert(stream->session->ctf_traces, &trace->ctf_trace_id, trace); @@ -398,8 +398,10 @@ int lttng_live_ctf_trace_assign(struct lttng_live_viewer_stream *stream, if (stream->metadata_flag) trace->metadata_stream = stream; + assert(!stream->in_trace); + stream->in_trace = 1; stream->ctf_trace = trace; - g_ptr_array_add(trace->streams, stream); + bt_list_add(&stream->trace_stream_node, &trace->stream_list); return ret; } @@ -546,7 +548,7 @@ int lttng_live_attach_session(struct lttng_live_ctx *ctx, uint64_t id) g_free(lvstream); goto error; } - bt_list_add(&lvstream->stream_node, + bt_list_add(&lvstream->session_stream_node, &ctx->session->stream_list); } ret = 0; @@ -1095,10 +1097,13 @@ retry: goto retry; case LTTNG_VIEWER_INDEX_HUP: printf_verbose("get_next_index: stream hung up\n"); - /* TODO: remove stream from session list and trace ptr array */ viewer_stream->id = -1ULL; index->offset = EOF; ctx->session->stream_count--; + viewer_stream->in_trace = 0; + bt_list_del(&viewer_stream->trace_stream_node); + bt_list_del(&viewer_stream->session_stream_node); + g_free(viewer_stream); break; case LTTNG_VIEWER_INDEX_ERR: fprintf(stderr, "[error] get_next_index: error\n"); @@ -1439,10 +1444,22 @@ int del_traces(gpointer key, gpointer value, gpointer user_data) struct bt_context *bt_ctx = user_data; struct lttng_live_ctf_trace *trace = value; int ret; + struct lttng_live_viewer_stream *lvstream, *tmp; - ret = bt_context_remove_trace(bt_ctx, trace->trace_id); - if (ret < 0) - fprintf(stderr, "[error] removing trace from context\n"); + /* + * We don't have ownership of the live viewer stream, just + * remove them from our list. + */ + bt_list_for_each_entry_safe(lvstream, tmp, &trace->stream_list, + trace_stream_node) { + lvstream->in_trace = 0; + bt_list_del(&lvstream->trace_stream_node); + } + if (trace->in_use) { + ret = bt_context_remove_trace(bt_ctx, trace->trace_id); + if (ret < 0) + fprintf(stderr, "[error] removing trace from context\n"); + } /* remove the key/value pair from the HT. */ return 1; @@ -1452,7 +1469,7 @@ static int add_one_trace(struct lttng_live_ctx *ctx, struct lttng_live_ctf_trace *trace) { - int i, ret; + int ret; struct bt_context *bt_ctx = ctx->bt_ctx; struct lttng_live_viewer_stream *stream; struct bt_mmap_stream *new_mmap_stream; @@ -1476,9 +1493,7 @@ int add_one_trace(struct lttng_live_ctx *ctx, BT_INIT_LIST_HEAD(&mmap_list.head); - for (i = 0; i < trace->streams->len; i++) { - stream = g_ptr_array_index(trace->streams, i); - + bt_list_for_each_entry(stream, &trace->stream_list, trace_stream_node) { if (!stream->metadata_flag) { new_mmap_stream = zmalloc(sizeof(struct bt_mmap_stream)); new_mmap_stream->priv = (void *) stream; @@ -1694,7 +1709,7 @@ int lttng_live_get_new_streams(struct lttng_live_ctx *ctx, uint64_t id) goto error; } nb_streams++; - bt_list_add(&lvstream->stream_node, + bt_list_add(&lvstream->session_stream_node, &ctx->session->stream_list); } ret = nb_streams; @@ -1726,7 +1741,7 @@ int lttng_live_read(struct lttng_live_ctx *ctx) fmt_write = bt_lookup_format(g_quark_from_static_string("text")); if (!fmt_write) { fprintf(stderr, "[error] ctf-text error\n"); - goto end; + goto end_free; } td_write = fmt_write->open_trace(NULL, O_RDWR, NULL, NULL); @@ -1796,10 +1811,10 @@ int lttng_live_read(struct lttng_live_ctx *ctx) if (!iter) { if (lttng_live_should_quit()) { ret = 0; - goto end; + goto end_free; } fprintf(stderr, "[error] Iterator creation error\n"); - goto end; + goto end_free; } for (;;) { if (lttng_live_should_quit()) { @@ -1832,6 +1847,8 @@ int lttng_live_read(struct lttng_live_ctx *ctx) } end_free: + g_hash_table_foreach_remove(ctx->session->ctf_traces, + del_traces, ctx->bt_ctx); bt_context_put(ctx->bt_ctx); end: if (lttng_live_should_quit()) { diff --git a/formats/lttng-live/lttng-live-plugin.c b/formats/lttng-live/lttng-live-plugin.c index a10a1b21..a7d015d1 100644 --- a/formats/lttng-live/lttng-live-plugin.c +++ b/formats/lttng-live/lttng-live-plugin.c @@ -220,8 +220,12 @@ static void free_session_streams(struct lttng_live_session *lsession) struct lttng_live_viewer_stream *lvstream, *tmp; bt_list_for_each_entry_safe(lvstream, tmp, &lsession->stream_list, - stream_node) { - bt_list_del(&lvstream->stream_node); + session_stream_node) { + /* + * The stream should not be in trace anymore. + */ + assert(!lvstream->in_trace); + bt_list_del(&lvstream->session_stream_node); g_free(lvstream); } } diff --git a/formats/lttng-live/lttng-live.h b/formats/lttng-live/lttng-live.h index ef799fd7..9739078a 100644 --- a/formats/lttng-live/lttng-live.h +++ b/formats/lttng-live/lttng-live.h @@ -66,7 +66,9 @@ struct lttng_live_viewer_stream { struct lttng_live_session *session; struct lttng_live_ctf_trace *ctf_trace; struct lttng_viewer_index current_index; - struct bt_list_head stream_node; + struct bt_list_head session_stream_node; /* Owns stream. */ + struct bt_list_head trace_stream_node; + int in_trace; char path[PATH_MAX]; }; @@ -74,6 +76,7 @@ struct lttng_live_session { uint64_t live_timer_interval; uint64_t stream_count; struct lttng_live_ctx *ctx; + /* The session stream list owns the lttng_live_viewer_stream object. */ struct bt_list_head stream_list; GHashTable *ctf_traces; }; @@ -81,7 +84,8 @@ struct lttng_live_session { struct lttng_live_ctf_trace { uint64_t ctf_trace_id; struct lttng_live_viewer_stream *metadata_stream; - GPtrArray *streams; + /* The trace has a list of streams, but it has no ownership on them. */ + struct bt_list_head stream_list; FILE *metadata_fp; struct bt_trace_handle *handle; int trace_id; -- 2.34.1