From 99d688f2849a22589351cf353edce0c756cddb74 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Thu, 22 Nov 2018 16:07:49 -0500 Subject: [PATCH] Wait for the destruction of sessions before tearing down main thread MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The main thread can no longer assume that it is the last thread to use the session objects (since they are now ref-counted). Hence, this commit introduces utils to allow the main thread to wait for the destruction of all sessions. The logic of the teardown is as follows: 1) The main thread waits for activity on the quit pipe. 2) Once the main thread unblocks, the main thread waits for the client thread to stop in order to guarantee that no new sessions are created. 3) The main thread then waits for the session list to be emptied 4) Once the session list is empty, continue the rest of the teardown as usual. Signed-off-by: Jérémie Galarneau --- src/bin/lttng-sessiond/lttng-sessiond.h | 3 + src/bin/lttng-sessiond/main.c | 73 +++++++++++++++++++------ src/bin/lttng-sessiond/session.c | 18 ++++++ src/bin/lttng-sessiond/session.h | 7 +++ src/bin/lttng-sessiond/thread-utils.c | 41 +++++++++++++- 5 files changed, 122 insertions(+), 20 deletions(-) diff --git a/src/bin/lttng-sessiond/lttng-sessiond.h b/src/bin/lttng-sessiond/lttng-sessiond.h index 25020d87e..9c5fcd32c 100644 --- a/src/bin/lttng-sessiond/lttng-sessiond.h +++ b/src/bin/lttng-sessiond/lttng-sessiond.h @@ -136,4 +136,7 @@ int sessiond_set_thread_pollset(struct lttng_poll_event *events, size_t size); void sessiond_notify_ready(void); void sessiond_signal_parents(void); +void sessiond_set_client_thread_state(bool running); +void sessiond_wait_client_thread_stopped(void); + #endif /* _LTT_SESSIOND_H */ diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index 98f586307..abae17fd6 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -375,7 +375,6 @@ static void wait_consumer(struct consumer_data *consumer_data) static void sessiond_cleanup(void) { int ret; - struct ltt_session *sess, *stmp; struct ltt_session_list *session_list = session_get_list(); DBG("Cleanup sessiond"); @@ -422,23 +421,7 @@ static void sessiond_cleanup(void) DBG("Removing directory %s", config.consumerd64_path.value); (void) rmdir(config.consumerd64_path.value); - DBG("Cleaning up all sessions"); - - /* Destroy session list mutex */ - if (session_list) { - session_lock_list(); - /* Cleanup ALL session */ - cds_list_for_each_entry_safe(sess, stmp, - &session_list->head, list) { - if (sess->destroyed) { - continue; - } - cmd_destroy_session(sess, - notification_thread_handle); - } - session_unlock_list(); - pthread_mutex_destroy(&session_list->lock); - } + pthread_mutex_destroy(&session_list->lock); wait_consumer(&kconsumer_data); wait_consumer(&ustconsumer64_data); @@ -4488,6 +4471,9 @@ static void *thread_manage_clients(void *data) health_code_update(); + /* Set state as running. */ + sessiond_set_client_thread_state(true); + while (1) { const struct cmd_completion_handler *cmd_completion_handler; @@ -4722,6 +4708,9 @@ error_create_poll: errno = ret; PERROR("join_consumer ust64"); } + + /* Set state as non-running. */ + sessiond_set_client_thread_state(false); return NULL; } @@ -5644,6 +5633,50 @@ end: return ret; } +static void destroy_all_sessions_and_wait(void) +{ + struct ltt_session *session, *tmp; + struct ltt_session_list *session_list; + + session_list = session_get_list(); + DBG("Initiating destruction of all sessions"); + + if (!session_list) { + return; + } + + /* + * Ensure that the client thread is no longer accepting new commands, + * which could cause new sessions to be created. + */ + sessiond_wait_client_thread_stopped(); + + session_lock_list(); + /* Initiate the destruction of all sessions. */ + cds_list_for_each_entry_safe(session, tmp, + &session_list->head, list) { + if (!session_get(session)) { + continue; + } + + session_lock(session); + if (session->destroyed) { + goto unlock_session; + } + (void) cmd_destroy_session(session, + notification_thread_handle); + unlock_session: + session_unlock(session); + session_put(session); + } + session_unlock_list(); + + /* Wait for the destruction of all sessions to complete. */ + DBG("Waiting for the destruction of all sessions to complete"); + session_list_wait_empty(); + DBG("Destruction of all sessions completed"); +} + /* * main */ @@ -6183,6 +6216,10 @@ int main(int argc, char **argv) PERROR("pthread_join load_session_thread"); retval = -1; } + + /* Initiate teardown once activity occurs on the quit pipe. */ + sessiond_wait_for_quit_pipe(-1U); + destroy_all_sessions_and_wait(); exit_load_session: if (is_root && !config.no_kernel) { diff --git a/src/bin/lttng-sessiond/session.c b/src/bin/lttng-sessiond/session.c index 173afa878..e7967f45e 100644 --- a/src/bin/lttng-sessiond/session.c +++ b/src/bin/lttng-sessiond/session.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -54,6 +55,7 @@ static struct ltt_session_list ltt_session_list = { .head = CDS_LIST_HEAD_INIT(ltt_session_list.head), .lock = PTHREAD_MUTEX_INITIALIZER, + .removal_cond = PTHREAD_COND_INITIALIZER, .next_uuid = 0, }; @@ -130,6 +132,19 @@ struct ltt_session_list *session_get_list(void) return <t_session_list; } +/* + * Returns once the session list is empty. + */ +void session_list_wait_empty(void) +{ + pthread_mutex_lock(<t_session_list.lock); + while (!cds_list_empty(<t_session_list.head)) { + pthread_cond_wait(<t_session_list.removal_cond, + <t_session_list.lock); + } + pthread_mutex_unlock(<t_session_list.lock); +} + /* * Acquire session list lock */ @@ -428,8 +443,11 @@ void session_release(struct urcu_ref *ref) consumer_output_put(session->consumer); snapshot_destroy(&session->snapshot); + + ASSERT_LOCKED(ltt_session_list.lock); del_session_list(session); del_session_ht(session); + pthread_cond_broadcast(<t_session_list.removal_cond); free(session); } diff --git a/src/bin/lttng-sessiond/session.h b/src/bin/lttng-sessiond/session.h index 6a059798d..ec0302920 100644 --- a/src/bin/lttng-sessiond/session.h +++ b/src/bin/lttng-sessiond/session.h @@ -47,6 +47,11 @@ struct ltt_session_list { * iterate or/and do any actions on that list. */ pthread_mutex_t lock; + /* + * This condition variable is signaled on every removal from + * the session list. + */ + pthread_cond_t removal_cond; /* * Session unique ID generator. The session list lock MUST be @@ -232,7 +237,9 @@ struct lttng_trace_archive_location *session_get_trace_archive_location( struct ltt_session *session_find_by_name(const char *name); struct ltt_session *session_find_by_id(uint64_t id); + struct ltt_session_list *session_get_list(void); +void session_list_wait_empty(void); int session_access_ok(struct ltt_session *session, uid_t uid, gid_t gid); diff --git a/src/bin/lttng-sessiond/thread-utils.c b/src/bin/lttng-sessiond/thread-utils.c index 99b829898..3a8ddb330 100644 --- a/src/bin/lttng-sessiond/thread-utils.c +++ b/src/bin/lttng-sessiond/thread-utils.c @@ -20,6 +20,9 @@ #include "lttng-sessiond.h" #include "utils.h" #include +#include + +#define USEC_PER_SEC 1000000 /* * Quit pipe for all threads. This permits a single cancellation point @@ -27,6 +30,19 @@ */ static int thread_quit_pipe[2] = { -1, -1 }; +/* + * Allows threads to query the state of the client thread. + */ +static struct client_thread_state { + pthread_cond_t cond; + pthread_mutex_t lock; + bool is_running; +} client_thread_state = { + .cond = PTHREAD_COND_INITIALIZER, + .lock = PTHREAD_MUTEX_INITIALIZER, + .is_running = false +}; + /* * Init thread quit pipe. * @@ -67,6 +83,8 @@ int sessiond_check_thread_quit_pipe(int fd, uint32_t events) /* * Wait for a notification on the quit pipe (with a timeout). * + * A timeout value of -1U means no timeout. + * * Returns 1 if the caller should quit, 0 if the timeout was reached, and * -1 if an error was encountered. */ @@ -79,11 +97,12 @@ int sessiond_wait_for_quit_pipe(unsigned int timeout_us) FD_ZERO(&read_fds); FD_SET(thread_quit_pipe[0], &read_fds); memset(&timeout, 0, sizeof(timeout)); - timeout.tv_usec = timeout_us; + timeout.tv_sec = timeout_us / USEC_PER_SEC; + timeout.tv_usec = timeout_us % USEC_PER_SEC; while (true) { ret = select(thread_quit_pipe[0] + 1, &read_fds, NULL, NULL, - &timeout); + timeout_us != -1U ? &timeout : NULL); if (ret < 0 && errno == EINTR) { /* Retry on interrupt. */ continue; @@ -117,6 +136,24 @@ void sessiond_close_quit_pipe(void) utils_close_pipe(thread_quit_pipe); } +void sessiond_set_client_thread_state(bool running) +{ + pthread_mutex_lock(&client_thread_state.lock); + client_thread_state.is_running = running; + pthread_cond_broadcast(&client_thread_state.cond); + pthread_mutex_unlock(&client_thread_state.lock); +} + +void sessiond_wait_client_thread_stopped(void) +{ + pthread_mutex_lock(&client_thread_state.lock); + while (client_thread_state.is_running) { + pthread_cond_wait(&client_thread_state.cond, + &client_thread_state.lock); + } + pthread_mutex_unlock(&client_thread_state.lock); +} + static int __sessiond_set_thread_pollset(struct lttng_poll_event *events, size_t size, int *a_pipe) -- 2.34.1