Add a StartupWaitTime parameter

This allows sesman to detect failed sessions before it tells xrdp
that all is OK with the session. This is a fairly common failure mode
which can now be reported on the login screen.

docs/man/sesman.ini.5.in

@@ -237,6 +237,16 @@ Note that the \fBU\fR and \fBB\fR criteria cannot be turned
 off. \fBDisplaySize\fR refers to the initial geometry of a connection,
 as actual display sizes can change dynamically.
 
+.TP
+\fBStartupWaitTime\fR=\fInumber\fR
+Milliseconds to wait to ensure the session has started. The default is 1500
+milli-seconds.
+.IP
+Making this larger does not increase the session startup time, but
+will increase the time for the first connection to a session.
+The value can be set to zero. If this is done, sessions which fail
+early will not be reported to the user.
+
 .SH "SECURITY"
 Following parameters can be used in the \fB[Security]\fR section.
 

libipm/scp_application_types.c

@@ -68,6 +68,7 @@ scp_screate_status_to_str(enum scp_screate_status n,
         (n == E_SCP_SCREATE_MAX_REACHED) ? "Max session limit reached" :
         (n == E_SCP_SCREATE_NO_DISPLAY) ? "No X displays are available" :
         (n == E_SCP_SCREATE_X_SERVER_FAIL) ? "X server could not be started" :
+        (n == E_SCP_SCREATE_SESSION_FAIL) ? "Session failed immediately" :
         (n == E_SCP_SCREATE_GENERAL_ERROR) ? "General session creation error" :
         /* Default */ NULL;
 

libipm/scp_application_types.h

@@ -98,6 +98,7 @@ enum scp_screate_status
     E_SCP_SCREATE_MAX_REACHED, ///< Max number of sessions already reached
     E_SCP_SCREATE_NO_DISPLAY, ///< No X server display number is available
     E_SCP_SCREATE_X_SERVER_FAIL, ///< X server could not be started
+    E_SCP_SCREATE_SESSION_FAIL,///< The session failed quickly
     E_SCP_SCREATE_GENERAL_ERROR ///< An unspecific error has occurred
 };
 

sesman/libsesman/sesman_config.c

@@ -81,6 +81,7 @@
 #define SESMAN_CFG_SESS_DISC_LIMIT   "DisconnectedTimeLimit"
 #define SESMAN_CFG_SESS_X11DISPLAYOFFSET "X11DisplayOffset"
 #define SESMAN_CFG_SESS_MAX_DISPLAY  "MaxDisplayNumber"
+#define SESMAN_CFG_SESS_STARTUP_WAIT_TIME "StartupWaitTime"
 
 #define SESMAN_CFG_SESS_POLICY_S "Policy"
 #define SESMAN_CFG_SESS_POLICY_DFLT_S "Default"
@@ -428,6 +429,7 @@ config_read_sessions(int file, struct config_sessions *se, struct list *param_n,
     se->max_disc_time = 0;
     se->kill_disconnected = 0;
     se->policy = SESMAN_CFG_SESS_POLICY_DEFAULT;
+    se->startup_wait_time = 1500;
 
     file_read_section(file, SESMAN_CFG_SESSIONS, param_n, param_v);
 
@@ -482,6 +484,20 @@ config_read_sessions(int file, struct config_sessions *se, struct list *param_n,
         {
             se->policy = parse_policy_string(value);
         }
+
+        else if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_STARTUP_WAIT_TIME))
+        {
+            int startup_wait_time = g_atoi(value);
+            if (startup_wait_time > 0 && startup_wait_time <= 15 * 1000)
+            {
+                se->startup_wait_time = startup_wait_time;
+            }
+            else
+            {
+                LOG(LOG_LEVEL_WARNING,
+                    "Ignoring bad " SESMAN_CFG_SESS_STARTUP_WAIT_TIME " value");
+            }
+        }
     }
 
     return 0;
@@ -673,6 +689,7 @@ config_dump(struct config_sesman *config)
     g_writeln("    IdleTimeLimit:            %d", se->max_idle_time);
     g_writeln("    DisconnectedTimeLimit:    %d", se->max_disc_time);
     g_writeln("    Policy:                   %s", policy_s);
+    g_writeln("    StartupWaitTime:          %u", se->startup_wait_time);
 
     /* Security configuration */
     g_writeln("Security configuration:");

sesman/libsesman/sesman_config.h

@@ -166,6 +166,11 @@ struct config_sessions
      * @brief session allocation policy
      */
     unsigned int policy;
+    /**
+     * @var start wait time
+     * @brief Wait time to make sure a session has started.
+     */
+    unsigned int startup_wait_time;
 };
 
 /**

sesman/sesexec/sesexec.c

@@ -266,39 +266,6 @@ sesexec_is_ecp_active(void)
 
 }
 
-/******************************************************************************/
-static void
-process_sigchld_event(void)
-{
-    struct proc_exit_status e;
-    int pid;
-
-    // Check for any finished children
-    while ((pid = g_waitchild(&e)) > 0)
-    {
-        session_process_child_exit(g_session_data, pid, &e);
-    }
-}
-
-/******************************************************************************/
-static void
-sesexec_terminate_session_and_wait(void)
-{
-    session_send_term(g_session_data);
-    do
-    {
-        g_sleep(1000);
-        // Process SIGCHLD events while waiting for the session
-        // to exit.
-        if (g_is_wait_obj_set(g_sigchld_event))
-        {
-            g_reset_wait_obj(g_sigchld_event);
-            process_sigchld_event();
-        }
-    }
-    while (session_active(g_session_data));
-}
-
 /******************************************************************************/
 static void
 sesexec_main_loop_cleanup(void)
@@ -314,7 +281,7 @@ sesexec_main_loop_cleanup(void)
     {
         LOG(LOG_LEVEL_INFO,
             "Stopping session on xrdp-sesexec exit");
-        sesexec_terminate_session_and_wait();
+        session_send_term(g_session_data, 1);
     }
     session_data_free(g_session_data);
 }
@@ -400,7 +367,7 @@ sesexec_main_loop(void)
             // See whether the session goes from active to inactive
             // after processing SIGCHLD
             int session_was_active = session_active(g_session_data);
-            process_sigchld_event();
+            session_process_sigchld_event(g_session_data);
             if (session_was_active && !session_active(g_session_data))
             {
                 // We've finished the session. Tell sesman and

sesman/sesexec/session.c

@@ -600,6 +600,65 @@ fork_child(
     return pid;
 }
 
+/******************************************************************************/
+static int
+process_startup_wait_time(struct session_data *sd)
+{
+    int rv = 0;
+    int robjs_count;
+    intptr_t robjs[10];
+    unsigned int start = g_get_elapsed_ms();
+
+    LOG(LOG_LEVEL_INFO, "Waiting for %u ms for session to start",
+        g_cfg->sess.startup_wait_time);
+    while (1)
+    {
+        unsigned int elapsed = g_get_elapsed_ms() - start;
+        if (elapsed >= g_cfg->sess.startup_wait_time)
+        {
+            break;
+        }
+
+        robjs_count = 0;
+        robjs[robjs_count++] = g_term_event;
+        robjs[robjs_count++] = g_sigchld_event;
+
+        if (g_obj_wait(robjs, robjs_count, NULL, 0,
+                       g_cfg->sess.startup_wait_time - elapsed) != 0)
+        {
+            /* should not get here */
+            LOG(LOG_LEVEL_WARNING, "process_startup_wait_time: "
+                "Unexpected error from g_obj_wait()");
+            g_sleep(100);
+            continue;
+        }
+
+        if (g_is_wait_obj_set(g_term_event)) /* term */
+        {
+            // Simulate success for now, but leave g_term_event set. The
+            // main loop will also pick up the terminate event and the
+            // session will be closed normally
+            break;
+        }
+
+        if (g_is_wait_obj_set(g_sigchld_event)) /* SIGCHLD */
+        {
+            g_reset_wait_obj(g_sigchld_event);
+            session_process_sigchld_event(sd);
+            if (sd->win_mgr < 0)
+            {
+                // Session has failed in the StartupWaitTime
+                // Wait for the rest of the session to finish
+                rv = 1;
+                session_send_term(sd, 1);
+                break;
+            }
+        }
+    }
+
+    return rv;
+}
+
 /******************************************************************************/
 static enum scp_screate_status
 session_start_wrapped(struct login_info *login_info,
@@ -709,17 +768,27 @@ session_start_wrapped(struct login_info *login_info,
                 chansrv_pid = fork_child(start_chansrv, login_info,
                                          s, display_pid);
 
-                // Tell the caller we've started
-                LOG(LOG_LEVEL_INFO,
-                    "Session in progress on display :%d. Waiting until the "
-                    "window manager (pid %d) exits to end the session",
-                    s->display, window_manager_pid);
-
                 sd->win_mgr = window_manager_pid;
                 sd->x_server = display_pid;
                 sd->chansrv = chansrv_pid;
                 sd->start_time = time(NULL);
-                status = E_SCP_SCREATE_OK;
+
+                if (process_startup_wait_time(sd) == 0)
+                {
+                    // Tell the caller we've started
+                    LOG(LOG_LEVEL_INFO,
+                        "Session in progress on display :%d. Waiting until the "
+                        "window manager (pid %d) exits to end the session",
+                        s->display, window_manager_pid);
+
+                    status = E_SCP_SCREATE_OK;
+                }
+                else
+                {
+                    LOG(LOG_LEVEL_ERROR,
+                        "Session failed during startup wait time");
+                    status = E_SCP_SCREATE_SESSION_FAIL;
+                }
             }
         }
     }
@@ -883,10 +952,19 @@ exit_status_to_str(const struct proc_exit_status *e, char buff[], int bufflen)
 }
 
 /******************************************************************************/
-void
+/**
-session_process_child_exit(struct session_data *sd,
+ * Processes an exited child
-                           int pid,
+ *
-                           const struct proc_exit_status *e)
+ * The PID of the child process is removed from the session_data.
+ *
+ * @param sd session_data for this session
+ * @param pid PID of exited process
+ * @param e Exit status of the exited process
+ */
+static void
+process_child_exit(struct session_data *sd,
+                   int pid,
+                   const struct proc_exit_status *e)
 {
     if (pid == sd->x_server)
     {
@@ -956,6 +1034,20 @@ session_process_child_exit(struct session_data *sd,
     }
 }
 
+/******************************************************************************/
+void
+session_process_sigchld_event(struct session_data *sd)
+{
+    struct proc_exit_status e;
+    int pid;
+
+    // Check for any finished children
+    while ((pid = g_waitchild(&e)) > 0)
+    {
+        process_child_exit(sd, pid, &e);
+    }
+}
+
 /******************************************************************************/
 unsigned int
 session_active(const struct session_data *sd)
@@ -982,14 +1074,34 @@ session_get_parameters(const struct session_data *sd)
 
 /******************************************************************************/
 void
-session_send_term(struct session_data *sd)
+session_send_term(struct session_data *sd, int wait_for_all)
 {
-    if (sd != NULL && sd->win_mgr > 0)
+    if (sd != NULL)
     {
-        // Killing the window manager only is appropriate here.
+        if (sd->win_mgr > 0)
-        // When we process SIGCHLD for the windowe manager, we
+        {
-        // will kill other processes as appropriate
+            // Killing the window manager only is appropriate here.
-        g_sigterm(sd->win_mgr);
+            // When we process SIGCHLD for the window manager, we
+            // will kill other processes as appropriate
+            g_sigterm(sd->win_mgr);
+        }
+
+        while (session_active(sd))
+        {
+            /* Don't check SIGTERM - we shouldn't be here long */
+            if (g_obj_wait(&g_sigchld_event, 1, NULL, 0, -1) != 0)
+            {
+                /* should not get here */
+                LOG(LOG_LEVEL_WARNING, "session_send_term: "
+                    "Unexpected error from g_obj_wait()");
+                g_sleep(100);
+            }
+            else
+            {
+                g_reset_wait_obj(g_sigchld_event);
+                session_process_sigchld_event(sd);
+            }
+        }
     }
 }
 

sesman/sesexec/session.h

@@ -78,18 +78,18 @@ session_start(struct login_info *login_info,
               struct session_data **session_data);
 
 /**
- * Processes an exited child process
+ * Processes a SIGCHLD event
  *
- * The PID of the child process is removed from the session_data.
+ * Any pending SIGCHLD events are processed.
+ *
+ * The PID of a failed child process is removed from the session_data.
  *
  * @param sd session_data for this session
  * @param pid PID of exited process
  * @param e Exit status of the exited process
  */
 void
-session_process_child_exit(struct session_data *sd,
+session_process_sigchld_event(struct session_data *sd);
-                           int pid,
-                           const struct proc_exit_status *e);
 
 /**
  * Returns a count of active processes in the session
@@ -124,9 +124,11 @@ session_get_parameters(const struct session_data *sd);
  * Ask a session to terminate by signalling the window manager
  *
  * @param sd session_data for this session
+ * @param wait_for_all != 0 to wait for all processes in the session
+ *                     to terminate
  */
 void
-session_send_term(struct session_data *sd);
+session_send_term(struct session_data *sd, int wait_for_all);
 
 /**
  * Frees a session_data object

sesman/sesman.ini.in

@@ -112,6 +112,10 @@ IdleTimeLimit=0
 ;   the string
 Policy=Default
 
+;; Startup wait time
+; Milliseconds to wait to ensure the session has started
+StartupWaitTime=1500
+
 [Logging]
 ; Note: Log levels can be any of: core, error, warning, info, debug, or trace
 LogFile=xrdp-sesman.log