From f056734385a493c37e45d1d5bcd65718dcac4f5d 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 Conflicts: src/bin/lttng-sessiond/ht-cleanup.c --- 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/ht-cleanup.c | 8 +++ src/bin/lttng-sessiond/jul-thread.c | 5 ++ src/bin/lttng-sessiond/main.c | 40 ++++++++++++ src/bin/lttng-sessiond/ust-thread.c | 5 ++ src/common/consumer.c | 8 ++- 9 files changed, 127 insertions(+), 30 deletions(-) diff --git a/src/bin/lttng-consumerd/health-consumerd.c b/src/bin/lttng-consumerd/health-consumerd.c index d1478e54d..e8afd963f 100644 --- a/src/bin/lttng-consumerd/health-consumerd.c +++ b/src/bin/lttng-consumerd/health-consumerd.c @@ -260,6 +260,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 8194c3e4b..402b44277 100644 --- a/src/bin/lttng-relayd/health-relayd.c +++ b/src/bin/lttng-relayd/health-relayd.c @@ -329,6 +329,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 a43ff7880..f86670e9c 100644 --- a/src/bin/lttng-relayd/live.c +++ b/src/bin/lttng-relayd/live.c @@ -514,6 +514,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) { @@ -1947,6 +1952,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 a1861de17..732309557 100644 --- a/src/bin/lttng-relayd/main.c +++ b/src/bin/lttng-relayd/main.c @@ -689,6 +689,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) { @@ -2356,6 +2361,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) { @@ -2458,46 +2468,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/ht-cleanup.c b/src/bin/lttng-sessiond/ht-cleanup.c index a201506c0..f343d620b 100644 --- a/src/bin/lttng-sessiond/ht-cleanup.c +++ b/src/bin/lttng-sessiond/ht-cleanup.c @@ -89,6 +89,14 @@ 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/jul-thread.c b/src/bin/lttng-sessiond/jul-thread.c index a683cdcdb..ad9f9f49f 100644 --- a/src/bin/lttng-sessiond/jul-thread.c +++ b/src/bin/lttng-sessiond/jul-thread.c @@ -305,6 +305,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/main.c b/src/bin/lttng-sessiond/main.c index 0a7f56827..c7beed55d 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -945,6 +945,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) { @@ -1090,6 +1095,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) { @@ -1217,6 +1227,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 @@ -1393,6 +1408,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) { @@ -1571,6 +1591,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 && @@ -1910,6 +1935,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) { @@ -3683,6 +3713,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) { @@ -3850,6 +3885,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 080afa53a..d0ea32480 100644 --- a/src/bin/lttng-sessiond/ust-thread.c +++ b/src/bin/lttng-sessiond/ust-thread.c @@ -91,6 +91,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 b5f9ae92e..aec7ba693 100644 --- a/src/common/consumer.c +++ b/src/common/consumer.c @@ -2210,6 +2210,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"); @@ -2780,10 +2785,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