Fix: checking for existing session daemon is done after daemonizing

The session daemon checks that no other session daemons are
running only after daemonizing. This means that launching the
deamon in background or daemon modes will appear to succeed even
if the launch failed due to an already present daemon.

The check is performed using both the client socket and the lock
file. This fix also addresses another problem that would cause
the pid file to be overwritten and deleted even if the session daemon
failed to launch.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 313e66e76cf965049fd5491410ae4c4c718a477a..5019b1c9c69a9e50c0d2dd0ccd1c2cf630f5ca20 100644 (file)
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -599,21 +599,6 @@ static void sessiond_cleanup(void)
                free(load_info);
        }
 
-       /*
-        * Cleanup lock file by deleting it and finaly closing it which will
-        * release the file system lock.
-        */
-       if (lockfile_fd >= 0) {
-               ret = remove(config.lock_file_path.value);
-               if (ret < 0) {
-                       PERROR("remove lock file");
-               }
-               ret = close(lockfile_fd);
-               if (ret < 0) {
-                       PERROR("close lock file");
-               }
-       }
-
        /*
         * We do NOT rmdir rundir because there are other processes
         * using it, for instance lttng-relayd, which can start in
@@ -5123,18 +5108,57 @@ end:
        return ret;
 }
 
+/*
+ * Create lockfile using the rundir and return its fd.
+ */
+static int create_lockfile(void)
+{
+        return utils_create_lock_file(config.lock_file_path.value);
+}
+
 /*
  * Check if the global socket is available, and if a daemon is answering at the
  * other side. If yes, error is returned.
+ *
+ * Also attempts to create and hold the lock file.
  */
 static int check_existing_daemon(void)
 {
+       int ret = 0;
+
        /* Is there anybody out there ? */
        if (lttng_session_daemon_alive()) {
-               return -EEXIST;
+               ret = -EEXIST;
+               goto end;
        }
 
-       return 0;
+       lockfile_fd = create_lockfile();
+       if (lockfile_fd < 0) {
+               ret = -EEXIST;
+               goto end;
+       }
+end:
+       return ret;
+}
+
+static void sessiond_cleanup_lock_file(void)
+{
+       int ret;
+
+       /*
+        * Cleanup lock file by deleting it and finaly closing it which will
+        * release the file system lock.
+        */
+       if (lockfile_fd >= 0) {
+               ret = remove(config.lock_file_path.value);
+               if (ret < 0) {
+                       PERROR("remove lock file");
+               }
+               ret = close(lockfile_fd);
+               if (ret < 0) {
+                       PERROR("close lock file");
+               }
+       }
 }
 
 /*
@@ -5393,14 +5417,6 @@ static int write_pidfile(void)
         return utils_create_pid_file(getpid(), config.pid_file_path.value);
 }
 
-/*
- * Create lockfile using the rundir and return its fd.
- */
-static int create_lockfile(void)
-{
-        return utils_create_lock_file(config.lock_file_path.value);
-}
-
 /*
  * Write agent TCP port using the rundir.
  */
@@ -5521,6 +5537,18 @@ int main(int argc, char **argv)
 
        sessiond_config_log(&config);
 
+       if (create_lttng_rundir()) {
+               retval = -1;
+               goto exit_options;
+       }
+
+       /* Abort launch if a session daemon is already running. */
+       if (check_existing_daemon()) {
+               ERR("A session daemon is already running.");
+               retval = -1;
+               goto exit_options;
+       }
+
        /* Daemonize */
        if (config.daemonize || config.background) {
                int i;
@@ -5535,9 +5563,12 @@ int main(int argc, char **argv)
                /*
                 * We are in the child. Make sure all other file descriptors are
                 * closed, in case we are called with more opened file
-                * descriptors than the standard ones.
+                * descriptors than the standard ones and the lock file.
                 */
                for (i = 3; i < sysconf(_SC_OPEN_MAX); i++) {
+                       if (i == lockfile_fd) {
+                               continue;
+                       }
                        (void) close(i);
                }
        }
@@ -5576,12 +5607,6 @@ int main(int argc, char **argv)
 
        /* Check if daemon is UID = 0 */
        is_root = !getuid();
-
-       if (create_lttng_rundir()) {
-               retval = -1;
-               goto exit_init_data;
-       }
-
        if (is_root) {
                /* Create global run dir with root access */
 
@@ -5600,12 +5625,6 @@ int main(int argc, char **argv)
                }
        }
 
-       lockfile_fd = create_lockfile();
-       if (lockfile_fd < 0) {
-               retval = -1;
-               goto exit_init_data;
-       }
-
        /* Set consumer initial state */
        kernel_consumerd_state = CONSUMER_STOPPED;
        ust_consumerd_state = CONSUMER_STOPPED;
@@ -5636,19 +5655,6 @@ int main(int argc, char **argv)
                goto exit_init_data;
        }
 
-       /*
-        * See if daemon already exist.
-        */
-       if (check_existing_daemon()) {
-               ERR("Already running daemon.\n");
-               /*
-                * We do not goto exit because we must not cleanup()
-                * because a daemon is already running.
-                */
-               retval = -1;
-               goto exit_init_data;
-       }
-
        /*
         * Init UST app hash table. Alloc hash table before this point since
         * cleanup() can get called after that point.
@@ -6060,6 +6066,7 @@ exit_health_sessiond_cleanup:
 exit_create_run_as_worker_cleanup:
 
 exit_options:
+       sessiond_cleanup_lock_file();
        sessiond_cleanup_options();
 
 exit_set_signal_handler: