From dbd6665b396de85a4d6b311c8942f82ea9813891 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Mon, 11 Nov 2019 23:38:25 -0500 Subject: [PATCH] Fix: relayd: session trace chunk is copied too late MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit In per-pid buffering mode, a viewer can attach to a session while it is active and find it has been closed by the time it requests new streams. The viewer session's trace chunk is created as a side-effect of the "get_new_streams" command and can find that the relay_session's trace chunk has now been closed, causing the creation of the viewer session trace chunk to fail. This results in an unexpected error being reported to the live client. This fix moves the creation of the viewer session's trace chunk to the "attach" command. If the creation fails, the session is reported as being "unknown". Reported-by: Francis Deslauriers Signed-off-by: Jérémie Galarneau Tested-by: Francis Deslauriers --- src/bin/lttng-relayd/live.c | 44 ++++++++------------------- src/bin/lttng-relayd/viewer-session.c | 32 +++++++++++++++---- src/bin/lttng-relayd/viewer-session.h | 3 +- 3 files changed, 41 insertions(+), 38 deletions(-) diff --git a/src/bin/lttng-relayd/live.c b/src/bin/lttng-relayd/live.c index 9a3d78ef0..3ca1797ed 100644 --- a/src/bin/lttng-relayd/live.c +++ b/src/bin/lttng-relayd/live.c @@ -291,6 +291,13 @@ static int make_viewer_streams(struct relay_session *session, assert(session); ASSERT_LOCKED(session->lock); + if (!viewer_trace_chunk) { + ERR("Internal error: viewer session associated with session \"%s\" has a NULL trace chunk", + session->session_name); + ret = -1; + goto error; + } + if (session->connection_closed) { *closed = true; } @@ -378,6 +385,7 @@ static int make_viewer_streams(struct relay_session *session, error_unlock: rcu_read_unlock(); +error: return ret; } @@ -959,25 +967,6 @@ int viewer_get_new_streams(struct relay_connection *conn) } pthread_mutex_lock(&session->lock); - if (!session->current_trace_chunk) { - /* - * Means the session is being destroyed. React the same way - * as if it could not be found at all. - */ - DBG("Relay session %" PRIu64 " has no current trace chunk, replying LTTNG_VIEWER_NEW_STREAMS_ERR", - session_id); - response.status = htobe32(LTTNG_VIEWER_NEW_STREAMS_ERR); - goto send_reply_unlock; - } - - if (!conn->viewer_session->current_trace_chunk && - session->current_trace_chunk) { - ret = viewer_session_set_trace_chunk(conn->viewer_session, - session->current_trace_chunk); - if (ret) { - goto error_unlock_session; - } - } ret = make_viewer_streams(session, conn->viewer_session->current_trace_chunk, LTTNG_VIEWER_SEEK_LAST, &nb_total, &nb_unsent, @@ -1057,6 +1046,7 @@ int viewer_attach_session(struct relay_connection *conn) struct lttng_viewer_attach_session_request request; struct lttng_viewer_attach_session_response response; struct relay_session *session = NULL; + enum lttng_viewer_attach_return_code viewer_attach_status; bool closed = false; uint64_t session_id; @@ -1106,10 +1096,10 @@ int viewer_attach_session(struct relay_connection *conn) } send_streams = 1; - ret = viewer_session_attach(conn->viewer_session, session); - if (ret) { - DBG("Already a viewer attached"); - response.status = htobe32(LTTNG_VIEWER_ATTACH_ALREADY); + viewer_attach_status = viewer_session_attach(conn->viewer_session, + session); + if (viewer_attach_status != LTTNG_VIEWER_ATTACH_OK) { + response.status = htobe32(viewer_attach_status); goto send_reply; } @@ -1126,14 +1116,6 @@ int viewer_attach_session(struct relay_connection *conn) goto send_reply; } - if (!conn->viewer_session->current_trace_chunk && - session->current_trace_chunk) { - ret = viewer_session_set_trace_chunk(conn->viewer_session, - session->current_trace_chunk); - if (ret) { - goto end_put_session; - } - } ret = make_viewer_streams(session, conn->viewer_session->current_trace_chunk, seek_type, &nb_streams, NULL, NULL, &closed); diff --git a/src/bin/lttng-relayd/viewer-session.c b/src/bin/lttng-relayd/viewer-session.c index 7ff58ed96..d394465ba 100644 --- a/src/bin/lttng-relayd/viewer-session.c +++ b/src/bin/lttng-relayd/viewer-session.c @@ -42,25 +42,45 @@ end: } /* The existence of session must be guaranteed by the caller. */ -int viewer_session_attach(struct relay_viewer_session *vsession, +enum lttng_viewer_attach_return_code viewer_session_attach( + struct relay_viewer_session *vsession, struct relay_session *session) { - int ret = 0; + enum lttng_viewer_attach_return_code viewer_attach_status = + LTTNG_VIEWER_ATTACH_OK; ASSERT_LOCKED(session->lock); /* Will not fail, as per the ownership guarantee. */ if (!session_get(session)) { - ret = -1; + viewer_attach_status = LTTNG_VIEWER_ATTACH_UNK; goto end; } if (session->viewer_attached) { - ret = -1; + viewer_attach_status = LTTNG_VIEWER_ATTACH_ALREADY; } else { + int ret; + + assert(session->current_trace_chunk); + assert(!vsession->current_trace_chunk); session->viewer_attached = true; + + ret = viewer_session_set_trace_chunk(vsession, + session->current_trace_chunk); + if (ret) { + /* + * The live protocol does not define a generic error + * value for the "attach" command. The "unknown" + * status is used so that the viewer may handle this + * failure as if the session didn't exist anymore. + */ + DBG("Failed to create a viewer trace chunk from the current trace chunk of session \"%s\", returning LTTNG_VIEWER_ATTACH_UNK", + session->session_name); + viewer_attach_status = LTTNG_VIEWER_ATTACH_UNK; + } } - if (!ret) { + if (viewer_attach_status == LTTNG_VIEWER_ATTACH_OK) { pthread_mutex_lock(&vsession->session_list_lock); /* Ownership is transfered to the list. */ cds_list_add_rcu(&session->viewer_session_node, @@ -71,7 +91,7 @@ int viewer_session_attach(struct relay_viewer_session *vsession, session_put(session); } end: - return ret; + return viewer_attach_status; } /* The existence of session must be guaranteed by the caller. */ diff --git a/src/bin/lttng-relayd/viewer-session.h b/src/bin/lttng-relayd/viewer-session.h index c127f69c1..5c8b2e54d 100644 --- a/src/bin/lttng-relayd/viewer-session.h +++ b/src/bin/lttng-relayd/viewer-session.h @@ -48,7 +48,8 @@ struct relay_viewer_session *viewer_session_create(void); void viewer_session_destroy(struct relay_viewer_session *vsession); void viewer_session_close(struct relay_viewer_session *vsession); -int viewer_session_attach(struct relay_viewer_session *vsession, +enum lttng_viewer_attach_return_code viewer_session_attach( + struct relay_viewer_session *vsession, struct relay_session *session); int viewer_session_is_attached(struct relay_viewer_session *vsession, struct relay_session *session); -- 2.34.1