From 6620da753247b1e0950c58ef3ce4f603c7da1ea5 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 9 Nov 2012 15:23:48 -0500 Subject: [PATCH] Fix: FD leak on thread error Also, don't spawn the kernel thread if not root and the no-kernel option is set. Fixes #316 Signed-off-by: David Goulet --- src/bin/lttng-relayd/main.c | 5 +-- src/bin/lttng-sessiond/cmd.c | 3 -- src/bin/lttng-sessiond/main.c | 81 ++++++++++++++++++++++------------- 3 files changed, 54 insertions(+), 35 deletions(-) diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c index 18e60d1ef..c099061cc 100644 --- a/src/bin/lttng-relayd/main.c +++ b/src/bin/lttng-relayd/main.c @@ -239,9 +239,6 @@ void cleanup(void) /* Close thread quit pipes */ utils_close_pipe(thread_quit_pipe); - /* Close relay cmd pipes */ - utils_close_pipe(relay_cmd_pipe); - uri_free(control_uri); uri_free(data_uri); } @@ -1813,6 +1810,8 @@ error_poll_create: streams_ht_error: lttng_ht_destroy(relay_connections_ht); relay_connections_ht_error: + /* Close relay cmd pipes */ + utils_close_pipe(relay_cmd_pipe); if (err) { DBG("Thread exited with error"); } diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c index 37a803b05..041b3e1bd 100644 --- a/src/bin/lttng-sessiond/cmd.c +++ b/src/bin/lttng-sessiond/cmd.c @@ -826,9 +826,6 @@ int cmd_enable_channel(struct ltt_session *session, { struct ltt_kernel_channel *kchan; - /* Mandatory for a kernel channel. */ - assert(wpipe > 0); - kchan = trace_kernel_get_channel_by_name(attr->name, session->kernel_session); if (kchan == NULL) { diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index fc52d759b..19eaa47d8 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -397,7 +397,7 @@ static void stop_threads(void) static void cleanup(void) { int ret; - char *cmd; + char *cmd = NULL; struct ltt_session *sess, *stmp; DBG("Cleaning up"); @@ -447,9 +447,6 @@ static void cleanup(void) modprobe_remove_lttng_all(); } - utils_close_pipe(kernel_poll_pipe); - utils_close_pipe(apps_cmd_pipe); - /* */ DBG("%c[%d;%dm*** assert failed :-) *** ==> %c[%dm%c[%d;%dm" "Matthew, BEET driven development works!%c[%dm", @@ -798,9 +795,13 @@ exit: error: lttng_poll_clean(&events); error_poll_create: + utils_close_pipe(kernel_poll_pipe); + kernel_poll_pipe[0] = kernel_poll_pipe[1] = -1; if (err) { health_error(&health_thread_kernel); ERR("Health error occurred in %s", __func__); + WARN("Kernel thread died unexpectedly. " + "Kernel tracing can continue but CPU hotplug is disabled."); } health_exit(&health_thread_kernel); DBG("Kernel thread dying"); @@ -1250,6 +1251,15 @@ exit: error: lttng_poll_clean(&events); error_poll_create: + utils_close_pipe(apps_cmd_pipe); + apps_cmd_pipe[0] = apps_cmd_pipe[1] = -1; + + /* + * We don't clean the UST app hash table here since already registered + * applications can still be controlled so let them be until the session + * daemon dies or the applications stop. + */ + if (err) { health_error(&health_thread_app_manage); ERR("Health error occurred in %s", __func__); @@ -1299,18 +1309,26 @@ static void *thread_dispatch_ust_registration(void *data) * call is blocking so we can be assured that the data will be read * at some point in time or wait to the end of the world :) */ - ret = write(apps_cmd_pipe[1], ust_cmd, - sizeof(struct ust_command)); - if (ret < 0) { - PERROR("write apps cmd pipe"); - if (errno == EBADF) { - /* - * We can't inform the application thread to process - * registration. We will exit or else application - * registration will not occur and tracing will never - * start. - */ - goto error; + if (apps_cmd_pipe[1] >= 0) { + ret = write(apps_cmd_pipe[1], ust_cmd, + sizeof(struct ust_command)); + if (ret < 0) { + PERROR("write apps cmd pipe"); + if (errno == EBADF) { + /* + * We can't inform the application thread to process + * registration. We will exit or else application + * registration will not occur and tracing will never + * start. + */ + goto error; + } + } + } else { + /* Application manager thread is not available. */ + ret = close(ust_cmd->sock); + if (ret < 0) { + PERROR("close ust_cmd sock"); } } free(ust_cmd); @@ -4043,8 +4061,10 @@ int main(int argc, char **argv) } /* Setup the kernel pipe for waking up the kernel thread */ - if ((ret = utils_create_pipe_cloexec(kernel_poll_pipe)) < 0) { - goto exit; + if (is_root && !opt_no_kernel) { + if ((ret = utils_create_pipe_cloexec(kernel_poll_pipe)) < 0) { + goto exit; + } } /* Setup the thread apps communication pipe. */ @@ -4134,18 +4154,21 @@ int main(int argc, char **argv) goto exit_apps; } - /* Create kernel thread to manage kernel event */ - ret = pthread_create(&kernel_thread, NULL, - thread_manage_kernel, (void *) NULL); - if (ret != 0) { - PERROR("pthread_create kernel"); - goto exit_kernel; - } + /* Don't start this thread if kernel tracing is not requested nor root */ + if (is_root && !opt_no_kernel) { + /* Create kernel thread to manage kernel event */ + ret = pthread_create(&kernel_thread, NULL, + thread_manage_kernel, (void *) NULL); + if (ret != 0) { + PERROR("pthread_create kernel"); + goto exit_kernel; + } - ret = pthread_join(kernel_thread, &status); - if (ret != 0) { - PERROR("pthread_join"); - goto error; /* join error, exit without cleanup */ + ret = pthread_join(kernel_thread, &status); + if (ret != 0) { + PERROR("pthread_join"); + goto error; /* join error, exit without cleanup */ + } } exit_kernel: -- 2.34.1