From 844a1607dae98620641d4587c5db4b044e43c476 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Thu, 30 May 2013 09:49:05 -0400 Subject: [PATCH] Fix: liblttng-ust process startup hang when sessiond is stopped Ensure the listener thread owns socket and notify_socket, so they don't have to hold the ust_lock() while connecting to the sessiond and reading from this socket. Therefore, after process fork, we can safely cleanup those retources, because the thread has been removed by the operating system. On exit, however, let the OS teardown those sockets, so exit path does not race with the listener thread. Fixes #545 Signed-off-by: Mathieu Desnoyers --- liblttng-ust/lttng-ust-comm.c | 53 ++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c index 4d437fc5..ae2e5189 100644 --- a/liblttng-ust/lttng-ust-comm.c +++ b/liblttng-ust/lttng-ust-comm.c @@ -625,6 +625,25 @@ void cleanup_sock_info(struct sock_info *sock_info, int exiting) { int ret; + if (sock_info->root_handle != -1) { + ret = lttng_ust_objd_unref(sock_info->root_handle, 1); + if (ret) { + ERR("Error unref root handle"); + } + sock_info->root_handle = -1; + } + sock_info->constructor_sem_posted = 0; + + /* + * wait_shm_mmap, socket and notify socket are used by listener + * threads outside of the ust lock, so we cannot tear them down + * ourselves, because we cannot join on these threads. Leave + * responsibility of cleaning up these resources to the OS + * process exit. + */ + if (exiting) + return; + if (sock_info->socket != -1) { ret = ustcomm_close_unix_sock(sock_info->socket); if (ret) { @@ -639,21 +658,7 @@ void cleanup_sock_info(struct sock_info *sock_info, int exiting) } sock_info->notify_socket = -1; } - if (sock_info->root_handle != -1) { - ret = lttng_ust_objd_unref(sock_info->root_handle, 1); - if (ret) { - ERR("Error unref root handle"); - } - sock_info->root_handle = -1; - } - sock_info->constructor_sem_posted = 0; - /* - * wait_shm_mmap is used by listener threads outside of the - * ust lock, so we cannot tear it down ourselves, because we - * cannot join on these threads. Leave this task to the OS - * process exit. - */ - if (!exiting && sock_info->wait_shm_mmap) { + if (sock_info->wait_shm_mmap) { ret = munmap(sock_info->wait_shm_mmap, sysconf(_SC_PAGE_SIZE)); if (ret) { ERR("Error unmapping wait shm"); @@ -910,11 +915,6 @@ restart: has_waited = 1; prev_connect_failed = 0; } - ust_lock(); - - if (lttng_ust_comm_should_quit) { - goto quit; - } if (sock_info->socket != -1) { ret = ustcomm_close_unix_sock(sock_info->socket); @@ -939,6 +939,13 @@ restart: if (ret < 0) { DBG("Info: sessiond not accepting connections to %s apps socket", sock_info->name); prev_connect_failed = 1; + + ust_lock(); + + if (lttng_ust_comm_should_quit) { + goto quit; + } + /* * If we cannot find the sessiond daemon, don't delay * constructor execution. @@ -976,6 +983,12 @@ restart: WARN("Unsuppoorted timeout value %ld", timeout); } + ust_lock(); + + if (lttng_ust_comm_should_quit) { + goto quit; + } + /* * Create only one root handle per listener thread for the whole * process lifetime, so we ensure we get ID which is statically -- 2.34.1