[pkg-fso-commits] [SCM] Automatic Display Manager branch, bug540201, updated. debian/0.6-1-52-g7a268a7

Enrico Zini enrico at enricozini.org
Wed Jul 6 19:13:33 UTC 2011


The following commit has been merged in the bug540201 branch:
commit 0d097f7b5a5f66fb555dbdea15c1778563b3941f
Author: Enrico Zini <enrico at enricozini.org>
Date:   Wed Jul 6 20:57:21 2011 +0200

    Block all signals in the monitor process by default, unblocking quit signals when we can catch it and exit

diff --git a/common.c b/common.c
index 897f90c..5a939ac 100644
--- a/common.c
+++ b/common.c
@@ -19,8 +19,12 @@
  */
 
 #include "common.h"
+#include "log.h"
 #include <stdlib.h>
 #include <string.h>
+#include <sys/wait.h>
+#include <errno.h>
+
 
 const char* basename (const char* str)
 {
@@ -51,14 +55,75 @@ const char* nodm_strerror(int code)
         case E_GSHADOW_NOTFOUND:   return "not found shadow group file";
         case E_CMD_NOEXEC:         return "can't run command/shell";
         case E_CMD_NOTFOUND:       return "can't find command/shell to run";
-        case E_PROGRAMMING:        return "Programming error";
+        case E_PROGRAMMING:        return "programming error";
         case E_PAM_ERROR:          return "something wrong talking with PAM";
         case E_OS_ERROR:           return "something wrong talking with the Operating System";
         case E_XLIB_ERROR:         return "Xlib error";
-        case E_X_SERVER_DIED:      return "Server died";
-        case E_X_SERVER_TIMEOUT:   return "Server not ready before timeout";
-        case E_X_SERVER_CONNECT:   return "Could not connect to X server";
+        case E_X_SERVER_DIED:      return "server died";
+        case E_X_SERVER_TIMEOUT:   return "server not ready before timeout";
+        case E_X_SERVER_CONNECT:   return "could not connect to X server";
         case E_SESSION_DIED:       return "X session died";
+        case E_USER_QUIT:          return "quit requested";
         default: return "unknown error";
     }
 }
+
+int child_has_quit(pid_t pid, int* quit, int* status)
+{
+    pid_t res = waitpid(pid, status, WNOHANG);
+    if (res == -1)
+    {
+        if (errno == ECHILD)
+            *quit = 2;
+        else
+        {
+            log_err("error checking status of child process %d: %m", (int)pid);
+            return E_OS_ERROR;
+        }
+    } else if (res == 0)
+        *quit = 0;
+    else
+        *quit = 1;
+    return E_SUCCESS;
+}
+
+int child_must_exit(pid_t pid, const char* procdesc)
+{
+    int res = E_SUCCESS;
+    if (pid > 0)
+    {
+        // Check what is the child status
+        int quit, status;
+        res = child_has_quit(pid, &quit, &status);
+        if (res != E_SUCCESS) return res;
+        switch (quit)
+        {
+            case 0:
+                // still running, we must kill it
+                log_info("sending %s %d the TERM signal", procdesc, (int)pid);
+                kill(pid, SIGTERM);
+                kill(pid, SIGCONT);
+                while (true)
+                {
+                    int status;
+                    if (waitpid(pid, &status, 0) == -1)
+                    {
+                        if (errno == EINTR)
+                            continue;
+                        if (errno != ECHILD)
+                            return E_OS_ERROR;
+                    }
+                    break;
+                }
+                break;
+            case 1:
+                // has just quit
+                log_info("%s %d quit with status %d", procdesc, (int)pid, status);
+                break;
+            case 2:
+                // Was not there
+                break;
+        }
+    }
+    return E_SUCCESS;
+}
diff --git a/common.h b/common.h
index fad3de6..c8c4bdf 100644
--- a/common.h
+++ b/common.h
@@ -21,6 +21,8 @@
 #ifndef NODM_DEFS_H
 #define NODM_DEFS_H
 
+#include <sys/types.h>
+
 // Exit codes used by shadow programs
 #define E_SUCCESS             0     ///< success
 #define E_NOPERM              1     ///< permission denied
@@ -43,6 +45,7 @@
 #define E_X_SERVER_TIMEOUT    211   ///< Server not ready before timeout
 #define E_X_SERVER_CONNECT    212   ///< Could not connect to X server
 #define E_SESSION_DIED        220   ///< X session died
+#define E_USER_QUIT           221   ///< Quit requested
 
 /// Return the basename of a path, as a pointer inside \a str
 const char* basename (const char* str);
@@ -64,4 +67,28 @@ const char* getenv_with_default(const char* envname, const char* def);
 /// Return the string description of an exit code
 const char* nodm_strerror(int code);
 
+/**
+ * Check if a child has died.
+ *
+ * @param pid
+ *   process ID to check
+ * @retval quit
+ *   0 if it is still running
+ *   1 if it has just quit and we got its exit status
+ *   2 if the pid does not exist
+ * @retval status
+ *   the exit status if it has quit
+ */
+int child_has_quit(pid_t pid, int* quit, int* status);
+
+/**
+ * Kill a child process if it still running and wait for it to end
+ *
+ * @param pid
+ *   The child pid
+ * @param procdesc
+ *   Child process description to use in the logs
+ */
+int child_must_exit(pid_t pid, const char* procdesc);
+
 #endif
diff --git a/dm.c b/dm.c
index 5a482d2..831fbfb 100644
--- a/dm.c
+++ b/dm.c
@@ -40,10 +40,20 @@ void nodm_display_manager_init(struct nodm_display_manager* dm)
     dm->conf_minimum_session_time = atoi(getenv_with_default("NODM_MIN_SESSION_TIME", "60"));
     dm->_srv_split_args = NULL;
     dm->_srv_split_argv = NULL;
+
+    // Save original signal mask
+    if (sigprocmask(SIG_BLOCK, NULL, &dm->orig_signal_mask) == -1)
+        log_err("sigprocmask error: %m");
+    dm->srv.orig_signal_mask = dm->orig_signal_mask;
+    dm->session.orig_signal_mask = dm->orig_signal_mask;
 }
 
 void nodm_display_manager_cleanup(struct nodm_display_manager* dm)
 {
+    // Restore original signal mask
+    if (sigprocmask(SIG_SETMASK, &dm->orig_signal_mask, NULL) == -1)
+        log_err("sigprocmask error: %m");
+
     nodm_vt_stop(&dm->vt);
 
     // Deallocate parsed arguments, if used
@@ -78,7 +88,18 @@ int nodm_display_manager_start(struct nodm_display_manager* dm)
         *s = NULL;
     }
 
-    dm->last_session_start = time(NULL);
+    // Block all signals
+    sigset_t blockmask;
+    if (sigfillset(&blockmask) == -1)
+    {
+        log_err("sigfillset error: %m");
+        return E_PROGRAMMING;
+    }
+    if (sigprocmask(SIG_BLOCK, &blockmask, NULL) == -1)
+    {
+        log_err("sigprocmask error: %m");
+        return E_PROGRAMMING;
+    }
 
     res = nodm_display_manager_restart(dm);
     if (res != E_SUCCESS) return res;
@@ -88,6 +109,8 @@ int nodm_display_manager_start(struct nodm_display_manager* dm)
 
 int nodm_display_manager_restart(struct nodm_display_manager* dm)
 {
+    dm->last_session_start = time(NULL);
+
     int res = nodm_xserver_start(&dm->srv);
     if (res != E_SUCCESS) return res;
 
@@ -108,8 +131,55 @@ int nodm_display_manager_stop(struct nodm_display_manager* dm)
     return E_SUCCESS;
 }
 
+// Signal handler for wait loop
+static int quit_signal_caught = 0;
+static void catch_signals (int sig)
+{
+    ++quit_signal_caught;
+}
+
+static int setup_quit_notification(sigset_t* origset)
+{
+    /* Reset caught signal flag */
+    quit_signal_caught = 0;
+
+    struct sigaction action;
+    action.sa_handler = catch_signals;
+    sigemptyset (&action.sa_mask);
+    action.sa_flags = 0;
+
+    sigset_t ourset;
+    if (sigemptyset(&ourset)
+        || sigaddset(&ourset, SIGTERM)
+        || sigaddset(&ourset, SIGINT)
+        || sigaddset(&ourset, SIGQUIT)
+        || sigaction(SIGTERM, &action, NULL)
+        || sigaction(SIGINT, &action, NULL)
+        || sigaction(SIGQUIT, &action, NULL)
+        || sigprocmask(SIG_UNBLOCK, &ourset, origset)
+        ) {
+        log_err("signal operations error: %m");
+        return E_PROGRAMMING;
+    }
+    return E_SUCCESS;
+}
+
+static void shutdown_quit_notification(const sigset_t* origset)
+{
+    if (sigprocmask(SIG_SETMASK, origset, NULL) == -1)
+        log_err("sigprocmask error: %m");
+}
+
+
 int nodm_display_manager_wait(struct nodm_display_manager* dm, int* session_status)
 {
+    int res = E_SUCCESS;
+
+    // Catch the normal termination signals using 'catch_signals'
+    sigset_t origset;
+    res = setup_quit_notification(&origset);
+    if (res != E_SUCCESS) return res;
+
     *session_status = -1;
     while (true)
     {
@@ -119,11 +189,20 @@ int nodm_display_manager_wait(struct nodm_display_manager* dm, int* session_stat
         if (child == -1)
         {
             if (errno == EINTR)
-                continue;
+            {
+                if (quit_signal_caught)
+                {
+                    res = E_USER_QUIT;
+                    goto cleanup;
+                }
+                else
+                    continue;
+            }
             else
             {
                 log_warn("waitpid error: %m");
-                return E_OS_ERROR;
+                res = E_OS_ERROR;
+                goto cleanup;
             }
         }
 
@@ -131,14 +210,20 @@ int nodm_display_manager_wait(struct nodm_display_manager* dm, int* session_stat
         {
             // Server died
             log_warn("X server died with status %d", status);
-            return E_X_SERVER_DIED;
+            res = E_X_SERVER_DIED;
+            goto cleanup;
         } else if (child == dm->session.pid) {
             // Session died
             log_warn("X session died with status %d", status);
             *session_status = status;
-            return E_SESSION_DIED;
+            res = E_SESSION_DIED;
+            goto cleanup;
         }
     }
+
+cleanup:
+    shutdown_quit_notification(&origset);
+    return res;
 }
 
 int nodm_display_manager_parse_xcmdline(struct nodm_display_manager* s, const char* xcmdline)
@@ -228,6 +313,42 @@ void nodm_display_manager_dump_status(struct nodm_display_manager* dm)
     nodm_xsession_dump_status(&dm->session);
 }
 
+static int interruptible_sleep(int seconds)
+{
+    int res = E_SUCCESS;
+
+    // Catch the normal termination signals using 'catch_signals'
+    sigset_t origset;
+    res = setup_quit_notification(&origset);
+    if (res != E_SUCCESS) return res;
+
+    struct timespec tosleep = { .tv_sec = seconds, .tv_nsec = 0 };
+    struct timespec remaining;
+    while (true)
+    {
+        int r = nanosleep(&tosleep, &remaining);
+        if (r != -1)
+            break;
+        else if (errno == EINTR)
+        {
+            if (quit_signal_caught)
+            {
+                res = E_USER_QUIT;
+                break;
+            } else
+                tosleep = remaining;
+        }
+        else
+        {
+            log_warn("sleep aborted: %m (ignoring error");
+            break;
+        }
+    }
+
+    shutdown_quit_notification(&origset);
+    return res;
+}
+
 int nodm_display_manager_wait_restart_loop(struct nodm_display_manager* dm)
 {
     static int retry_times[] = { 0, 0, 30, 30, 60, 60, -1 };
@@ -246,7 +367,6 @@ int nodm_display_manager_wait_restart_loop(struct nodm_display_manager* dm)
             case E_X_SERVER_DIED:
                 break;
             case E_SESSION_DIED:
-                log_info("X session quit with status %d", sstatus);
                 break;
             default:
                 return res;
@@ -266,21 +386,8 @@ int nodm_display_manager_wait_restart_loop(struct nodm_display_manager* dm)
         {
             log_warn("session lasted less than %d seconds: sleeping %d seconds before restarting it",
                     dm->conf_minimum_session_time, retry_times[restart_count]);
-            struct timespec tosleep = { .tv_sec = retry_times[restart_count], .tv_nsec = 0 };
-            struct timespec remaining;
-            while (true)
-            {
-                int r = nanosleep(&tosleep, &remaining);
-                if (r != -1)
-                    break;
-                else if (errno == EINTR)
-                    tosleep = remaining;
-                else
-                {
-                    log_warn("sleep aborted: %m (ignoring error");
-                    break;
-                }
-            }
+            res = interruptible_sleep(retry_times[restart_count]);
+            if (res != E_SUCCESS) return res;
         }
 
         log_info("restarting session");
diff --git a/dm.h b/dm.h
index 051965e..04d0e8f 100644
--- a/dm.h
+++ b/dm.h
@@ -25,6 +25,7 @@
 #include "xsession.h"
 #include "vt.h"
 #include <time.h>
+#include <signal.h>
 
 struct nodm_display_manager
 {
@@ -46,6 +47,9 @@ struct nodm_display_manager
     /// Time the last session started
     time_t last_session_start;
 
+    /// Original signal mask at program startup
+    sigset_t orig_signal_mask;
+
     /// Storage for split server arguments used by nodm_x_cmdline_split
     char** _srv_split_argv;
     void* _srv_split_args;
@@ -60,7 +64,12 @@ void nodm_display_manager_init(struct nodm_display_manager* dm);
 /// Cleanup at the end of the display manager
 void nodm_display_manager_cleanup(struct nodm_display_manager* dm);
 
-/// Start X and the X session
+/**
+ * Start X and the X session
+ *
+ * This function sets the signal mask to block all signals. The original signal
+ * mask is restored by nodm_display_manager_cleanup().
+ */
 int nodm_display_manager_start(struct nodm_display_manager* dm);
 
 /// Restart X and the X session after they died
diff --git a/xserver.c b/xserver.c
index cbffcc1..2829c5b 100644
--- a/xserver.c
+++ b/xserver.c
@@ -77,12 +77,16 @@ void nodm_xserver_init(struct nodm_xserver* srv)
     srv->pid = -1;
     srv->dpy = NULL;
     srv->windowpath = NULL;
+    if (sigemptyset(&srv->orig_signal_mask) == -1)
+        log_err("sigemptyset error: %m");
 }
 
 int nodm_xserver_start(struct nodm_xserver* srv)
 {
     // Function return code
     int return_code = E_SUCCESS;
+    // True if we should restore the signal mask at exit
+    bool signal_mask_altered = false;
 
     // Initialise common signal handling machinery
     struct sigaction sa;
@@ -113,8 +117,15 @@ int nodm_xserver_start(struct nodm_xserver* srv)
     // From now on we need to perform cleanup before returning
 
     // fork/exec the X server
-    pid_t child = fork ();
-    if (child == 0) {   /* child shell */
+    srv->pid = fork ();
+    if (srv->pid == 0)
+    {
+        // child
+
+        // Restore the original signal mask
+        if (sigprocmask(SIG_SETMASK, &srv->orig_signal_mask, NULL) == -1)
+            log_err("sigprocmask failed: %m");
+
         // Stop the logging subsystem before we quit via exec
         log_end();
 
@@ -125,7 +136,7 @@ int nodm_xserver_start(struct nodm_xserver* srv)
         execv(srv->argv[0], (char *const*)srv->argv);
         log_err("cannot start %s: %m", srv->argv[0]);
         exit(errno == ENOENT ? E_CMD_NOTFOUND : E_CMD_NOEXEC);
-    } else if (child == -1) {
+    } else if (srv->pid == -1) {
         log_err("cannot fork to run %s: %m", srv->argv[0]);
         return_code = E_OS_ERROR;
         goto cleanup;
@@ -142,13 +153,36 @@ int nodm_xserver_start(struct nodm_xserver* srv)
         goto cleanup;
     }
 
+    // Unblock SIGCHLD and SIGUSR1
+    sigset_t orig_set;
+    sigset_t cur_set;
+    if (sigemptyset(&cur_set) == -1)
+    {
+        log_err("sigemptyset failed: %m");
+        return_code = E_PROGRAMMING;
+        goto cleanup;
+    }
+    if (sigaddset(&cur_set, SIGCHLD) == -1 || sigaddset(&cur_set, SIGUSR1) == -1)
+    {
+        log_err("sigaddset failed: %m");
+        return_code = E_PROGRAMMING;
+        goto cleanup;
+    }
+    if (sigprocmask(SIG_UNBLOCK, &cur_set, &orig_set) == -1)
+    {
+        log_err("sigprocmask failed: %m");
+        return_code = E_PROGRAMMING;
+        goto cleanup;
+    }
+    signal_mask_altered = true;
+
     // Wait for SIGUSR1, for the server to die or for a timeout
     struct timespec timeout = { .tv_sec = srv->conf_timeout, .tv_nsec = 0 };
     while (!server_started)
     {
         // Check if the server has died
         int status;
-        pid_t res = waitpid(child, &status, WNOHANG);
+        pid_t res = waitpid(srv->pid, &status, WNOHANG);
         if (res == -1)
         {
             if (errno == EINTR) continue;
@@ -156,7 +190,7 @@ int nodm_xserver_start(struct nodm_xserver* srv)
             return_code = E_OS_ERROR;
             goto cleanup;
         }
-        if (res == child)
+        if (res == srv->pid)
         {
             if (WIFEXITED(status))
                 log_err("X server exited with status=%d", WEXITSTATUS(status));
@@ -166,6 +200,7 @@ int nodm_xserver_start(struct nodm_xserver* srv)
                 // This should never happen, but it's better to have a message
                 // than to fail silently through an open code path
                 log_err("X server quit, waitpid gave unrecognised status=%d", status);
+            srv->pid = -1;
             return_code = E_X_SERVER_DIED;
             goto cleanup;
         }
@@ -192,11 +227,15 @@ int nodm_xserver_start(struct nodm_xserver* srv)
     log_verbose("X is ready to accept connections");
 
 cleanup:
+    // Restore signal mask
+    if (signal_mask_altered)
+        if (sigprocmask(SIG_SETMASK, &orig_set, NULL) == -1)
+            log_err("sigprocmask failed: %m");
+
     // Kill the X server if an error happened
-    if (child > 0 && return_code != E_SUCCESS)
-        kill(child, SIGTERM);
-    else
-        srv->pid = child;
+    if (srv->pid > 0 && return_code != E_SUCCESS)
+        nodm_xserver_stop(srv);
+
     // Restore signal handlers
     sigaction(SIGCHLD, NULL, &sa_sigchld_old);
     sigaction(SIGUSR1, NULL, &sa_usr1_old);
@@ -205,24 +244,7 @@ cleanup:
 
 int nodm_xserver_stop(struct nodm_xserver* srv)
 {
-    int res = E_SUCCESS;
-    if (srv->pid > 0)
-    {
-        kill(srv->pid, SIGTERM);
-        kill(srv->pid, SIGCONT);
-        while (true)
-        {
-            int status;
-            if (waitpid(srv->pid, &status, 0) == -1)
-            {
-                if (errno == EINTR)
-                    continue;
-                if (errno != ECHILD)
-                    res = E_OS_ERROR;
-            }
-            break;
-        }
-    }
+    int res = child_must_exit(srv->pid, "X server");
     srv->pid = -1;
 
     if (srv->windowpath != NULL)
@@ -252,12 +274,21 @@ int nodm_xserver_connect(struct nodm_xserver* srv)
 {
     //XSetErrorHandler(x_error_handler);
 
-    XSetIOErrorHandler(xopendisplay_error_handler);
-    srv->dpy = XOpenDisplay(srv->name);
-    XSetIOErrorHandler((int (*)(Display *))0);
+    for (int i = 0; i < 5; ++i)
+    {
+        if (i > 0)
+            log_info("connecting to X server, attempt #%d", i+1);
+        XSetIOErrorHandler(xopendisplay_error_handler);
+        srv->dpy = XOpenDisplay(srv->name);
+        XSetIOErrorHandler((int (*)(Display *))0);
 
-    if (srv->dpy == NULL)
-        log_err("could not connect to X server on \"%s\"", srv->name);
+        if (srv->dpy == NULL)
+        {
+            log_err("could not connect to X server on \"%s\"", srv->name);
+            sleep(1);
+        } else
+            break;
+    }
 
     // from xdm: remove close-on-exec and register to close at the next fork, why? We'll find out
     // RegisterCloseOnFork (ConnectionNumber (d->dpy));
diff --git a/xserver.h b/xserver.h
index 6c701a8..efddcc2 100644
--- a/xserver.h
+++ b/xserver.h
@@ -40,6 +40,8 @@ struct nodm_xserver
     pid_t pid;
     /// xlib Display connected to the server
     Display *dpy;
+    /// Original signal mask at program startup
+    sigset_t orig_signal_mask;
 };
 
 /**
diff --git a/xsession.c b/xsession.c
index 45cfcdb..9f6acf5 100644
--- a/xsession.c
+++ b/xsession.c
@@ -39,6 +39,9 @@ int nodm_xsession_init(struct nodm_xsession* s)
     s->conf_use_pam = true;
     s->conf_cleanup_xse = true;
 
+    if (sigemptyset(&s->orig_signal_mask) == -1)
+        log_err("sigemptyset error: %m");
+
     // Get the user we should run the session for
     if (!bounded_strcpy(s->conf_run_as, getenv_with_default("NODM_USER", "root")))
         log_warn("username has been truncated");
@@ -111,6 +114,10 @@ int nodm_xsession_start(struct nodm_xsession* s, struct nodm_xserver* srv)
     s->pid = fork();
     if (s->pid == 0)
     {
+        // Restore the original signal mask
+        if (sigprocmask(SIG_SETMASK, &s->orig_signal_mask, NULL) == -1)
+            log_err("sigprocmask failed: %m");
+
         // child shell */
         if (s->child_body)
             exit(s->child_body(&child));
@@ -128,25 +135,7 @@ int nodm_xsession_start(struct nodm_xsession* s, struct nodm_xserver* srv)
 
 int nodm_xsession_stop(struct nodm_xsession* s)
 {
-    int res = E_SUCCESS;
-
-    if (s->pid > 0)
-    {
-        kill(s->pid, SIGTERM);
-        kill(s->pid, SIGCONT);
-        while (true)
-        {
-            int status;
-            if (waitpid(s->pid, &status, 0) == -1)
-            {
-                if (errno == EINTR)
-                    continue;
-                if (errno != ECHILD)
-                    res = E_OS_ERROR;
-            }
-            break;
-        }
-    }
+    int res = child_must_exit(s->pid, "X session");
     s->pid = -1;
     return res;
 }
diff --git a/xsession.h b/xsession.h
index f5ce321..b095d8e 100644
--- a/xsession.h
+++ b/xsession.h
@@ -51,6 +51,9 @@ struct nodm_xsession
 
     /// If non-NULL, use as child process main body (used for tests)
     int (*child_body)(struct nodm_xsession_child* s);
+
+    /// Original signal mask at program startup
+    sigset_t orig_signal_mask;
 };
 
 /// Initialise a struct nodm_session with default values

-- 
Automatic Display Manager



More information about the pkg-fso-commits mailing list