From fd20dac985126e84929d657f5a1042222c7d5017 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 5 Jan 2015 16:43:05 -0500 Subject: [PATCH] Fix: compat poll: add missing empty revents checks MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Poll returns the entire array, including entries that have no activity. We need to check them explicitly. Fixes #747 Signed-off-by: Mathieu Desnoyers Signed-off-by: Jérémie Galarneau --- src/bin/lttng-consumerd/health-consumerd.c | 5 ++ src/bin/lttng-relayd/health-relayd.c | 5 ++ src/bin/lttng-relayd/live.c | 10 +++ src/bin/lttng-relayd/main.c | 71 +++++++++++++--------- src/bin/lttng-sessiond/agent-thread.c | 5 ++ src/bin/lttng-sessiond/ht-cleanup.c | 10 +++ src/bin/lttng-sessiond/main.c | 40 ++++++++++++ src/bin/lttng-sessiond/ust-thread.c | 5 ++ src/common/consumer.c | 8 ++- 9 files changed, 129 insertions(+), 30 deletions(-) diff --git a/src/bin/lttng-consumerd/health-consumerd.c b/src/bin/lttng-consumerd/health-consumerd.c index bd47f0c49..fc9a26665 100644 --- a/src/bin/lttng-consumerd/health-consumerd.c +++ b/src/bin/lttng-consumerd/health-consumerd.c @@ -261,6 +261,11 @@ restart: revents = LTTNG_POLL_GETEV(&events, i); pollfd = LTTNG_POLL_GETFD(&events, i); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = check_health_quit_pipe(pollfd, revents); if (ret) { diff --git a/src/bin/lttng-relayd/health-relayd.c b/src/bin/lttng-relayd/health-relayd.c index af375e26d..75149ba4b 100644 --- a/src/bin/lttng-relayd/health-relayd.c +++ b/src/bin/lttng-relayd/health-relayd.c @@ -330,6 +330,11 @@ restart: revents = LTTNG_POLL_GETEV(&events, i); pollfd = LTTNG_POLL_GETFD(&events, i); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = check_health_quit_pipe(pollfd, revents); if (ret) { diff --git a/src/bin/lttng-relayd/live.c b/src/bin/lttng-relayd/live.c index fd570e0fc..756efbede 100644 --- a/src/bin/lttng-relayd/live.c +++ b/src/bin/lttng-relayd/live.c @@ -488,6 +488,11 @@ restart: revents = LTTNG_POLL_GETEV(&events, i); pollfd = LTTNG_POLL_GETFD(&events, i); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = check_thread_quit_pipe(pollfd, revents); if (ret) { @@ -1922,6 +1927,11 @@ restart: health_code_update(); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = check_thread_quit_pipe(pollfd, revents); if (ret) { diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c index 1ec1eeaa4..94bb4cb8e 100644 --- a/src/bin/lttng-relayd/main.c +++ b/src/bin/lttng-relayd/main.c @@ -868,6 +868,11 @@ restart: revents = LTTNG_POLL_GETEV(&events, i); pollfd = LTTNG_POLL_GETFD(&events, i); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = check_thread_quit_pipe(pollfd, revents); if (ret) { @@ -2536,6 +2541,11 @@ restart: health_code_update(); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = check_thread_quit_pipe(pollfd, revents); if (ret) { @@ -2638,46 +2648,49 @@ restart: health_code_update(); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Skip the command pipe. It's handled in the first loop. */ if (pollfd == relay_conn_pipe[0]) { continue; } - if (revents) { - rcu_read_lock(); - conn = connection_find_by_sock(relay_connections_ht, pollfd); - if (!conn) { - /* Skip it. Might be removed before. */ + rcu_read_lock(); + conn = connection_find_by_sock(relay_connections_ht, pollfd); + if (!conn) { + /* Skip it. Might be removed before. */ + rcu_read_unlock(); + continue; + } + + if (revents & LPOLLIN) { + if (conn->type != RELAY_DATA) { rcu_read_unlock(); continue; } - if (revents & LPOLLIN) { - if (conn->type != RELAY_DATA) { - rcu_read_unlock(); - continue; - } - - ret = relay_process_data(conn); - /* Connection closed */ - if (ret < 0) { - cleanup_connection_pollfd(&events, pollfd); - destroy_connection(relay_connections_ht, conn); - DBG("Data connection closed with %d", pollfd); - /* - * Every goto restart call sets the last seen fd where - * here we don't really care since we gracefully - * continue the loop after the connection is deleted. - */ - } else { - /* Keep last seen port. */ - last_seen_data_fd = pollfd; - rcu_read_unlock(); - goto restart; - } + ret = relay_process_data(conn); + /* Connection closed */ + if (ret < 0) { + cleanup_connection_pollfd(&events, pollfd); + destroy_connection(relay_connections_ht, conn); + DBG("Data connection closed with %d", pollfd); + /* + * Every goto restart call sets the last seen fd where + * here we don't really care since we gracefully + * continue the loop after the connection is deleted. + */ + } else { + /* Keep last seen port. */ + last_seen_data_fd = pollfd; + rcu_read_unlock(); + goto restart; } - rcu_read_unlock(); } + rcu_read_unlock(); } last_seen_data_fd = -1; } diff --git a/src/bin/lttng-sessiond/agent-thread.c b/src/bin/lttng-sessiond/agent-thread.c index d7d9c6c34..a9cc6e735 100644 --- a/src/bin/lttng-sessiond/agent-thread.c +++ b/src/bin/lttng-sessiond/agent-thread.c @@ -330,6 +330,11 @@ restart: revents = LTTNG_POLL_GETEV(&events, i); pollfd = LTTNG_POLL_GETFD(&events, i); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = sessiond_check_thread_quit_pipe(pollfd, revents); if (ret) { diff --git a/src/bin/lttng-sessiond/ht-cleanup.c b/src/bin/lttng-sessiond/ht-cleanup.c index ace41e5cd..373c913dc 100644 --- a/src/bin/lttng-sessiond/ht-cleanup.c +++ b/src/bin/lttng-sessiond/ht-cleanup.c @@ -96,6 +96,11 @@ restart: revents = LTTNG_POLL_GETEV(&events, i); pollfd = LTTNG_POLL_GETFD(&events, i); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + if (pollfd != ht_cleanup_pipe[0]) { continue; } @@ -140,6 +145,11 @@ restart: revents = LTTNG_POLL_GETEV(&events, i); pollfd = LTTNG_POLL_GETFD(&events, i); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + if (pollfd == ht_cleanup_pipe[0]) { continue; } diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index 0dbec232f..80eebf163 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -1090,6 +1090,11 @@ static void *thread_manage_kernel(void *data) health_code_update(); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = sessiond_check_thread_quit_pipe(pollfd, revents); if (ret) { @@ -1235,6 +1240,11 @@ restart: health_code_update(); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = sessiond_check_thread_quit_pipe(pollfd, revents); if (ret) { @@ -1362,6 +1372,11 @@ restart_poll: health_code_update(); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* * Thread quit pipe has been triggered, flag that we should stop * but continue the current loop to handle potential data from @@ -1538,6 +1553,11 @@ static void *thread_manage_apps(void *data) health_code_update(); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = sessiond_check_thread_quit_pipe(pollfd, revents); if (ret) { @@ -1716,6 +1736,11 @@ static void sanitize_wait_queue(struct ust_reg_wait_queue *wait_queue) uint32_t revents = LTTNG_POLL_GETEV(&events, i); int pollfd = LTTNG_POLL_GETFD(&events, i); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + cds_list_for_each_entry_safe(wait_node, tmp_wait_node, &wait_queue->head, head) { if (pollfd == wait_node->app->sock && @@ -2055,6 +2080,11 @@ static void *thread_registration_apps(void *data) revents = LTTNG_POLL_GETEV(&events, i); pollfd = LTTNG_POLL_GETFD(&events, i); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = sessiond_check_thread_quit_pipe(pollfd, revents); if (ret) { @@ -3918,6 +3948,11 @@ restart: revents = LTTNG_POLL_GETEV(&events, i); pollfd = LTTNG_POLL_GETFD(&events, i); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = sessiond_check_thread_quit_pipe(pollfd, revents); if (ret) { @@ -4090,6 +4125,11 @@ static void *thread_manage_clients(void *data) health_code_update(); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = sessiond_check_thread_quit_pipe(pollfd, revents); if (ret) { diff --git a/src/bin/lttng-sessiond/ust-thread.c b/src/bin/lttng-sessiond/ust-thread.c index d92c1f908..6f4295db7 100644 --- a/src/bin/lttng-sessiond/ust-thread.c +++ b/src/bin/lttng-sessiond/ust-thread.c @@ -92,6 +92,11 @@ restart: revents = LTTNG_POLL_GETEV(&events, i); pollfd = LTTNG_POLL_GETFD(&events, i); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + /* Thread quit pipe has been closed. Killing thread. */ ret = sessiond_check_thread_quit_pipe(pollfd, revents); if (ret) { diff --git a/src/common/consumer.c b/src/common/consumer.c index d192d7a51..0035aae82 100644 --- a/src/common/consumer.c +++ b/src/common/consumer.c @@ -2212,6 +2212,11 @@ restart: revents = LTTNG_POLL_GETEV(&events, i); pollfd = LTTNG_POLL_GETFD(&events, i); + if (!revents) { + /* No activity for this FD (poll implementation). */ + continue; + } + if (pollfd == lttng_pipe_get_readfd(ctx->consumer_metadata_pipe)) { if (revents & (LPOLLERR | LPOLLHUP )) { DBG("Metadata thread pipe hung up"); @@ -2782,10 +2787,11 @@ restart: revents = LTTNG_POLL_GETEV(&events, i); pollfd = LTTNG_POLL_GETFD(&events, i); - /* Just don't waste time if no returned events for the fd */ if (!revents) { + /* No activity for this FD (poll implementation). */ continue; } + if (pollfd == ctx->consumer_channel_pipe[0]) { if (revents & (LPOLLERR | LPOLLHUP)) { DBG("Channel thread pipe hung up"); -- 2.34.1