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 <dgoulet@efficios.com>

diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c
index 18e60d1efbbe40fb45c678adc149b4e3c366468e..c099061ccf7339f03741fc3cda59fb5ae60901a6 100644 (file)
--- 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 37a803b05ff12d399bb9d6de6a1c8bafe9fb2b2f..041b3e1bda8ea5e5d4c678aa26f14d0b7d831e38 100644 (file)
--- 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 fc52d759b2130b8f1bdd2e122f24c1990f9b85d2..19eaa47d88b1fe38cdc6a56313aac7438f153f8f 100644 (file)
--- 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);
-
        /* <fun> */
        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: