Log xrdp sub-processes which fail due to a signal

xrdp/xrdp.c

@@ -102,9 +102,7 @@ xrdp_shutdown(int sig)
 static void
 xrdp_child(int sig)
 {
-    while (g_waitchild(NULL) > 0)
-    {
-    }
+    g_set_sigchld(1);
 }
 
 /*****************************************************************************/
@@ -562,6 +560,14 @@ main(int argc, char **argv)
         LOG(LOG_LEVEL_WARNING, "error creating g_term_event");
     }
 
+    g_snprintf(text, 255, "xrdp_%8.8x_main_sigchld", pid);
+    g_set_sigchld_event(g_create_wait_obj(text));
+
+    if (g_get_sigchld() == 0)
+    {
+        LOG(LOG_LEVEL_WARNING, "error creating g_sigchld_event");
+    }
+
     g_snprintf(text, 255, "xrdp_%8.8x_main_sync", pid);
     g_set_sync_event(g_create_wait_obj(text));
 
@@ -583,6 +589,9 @@ main(int argc, char **argv)
     g_delete_wait_obj(g_get_term());
     g_set_term_event(0);
 
+    g_delete_wait_obj(g_get_sigchld());
+    g_set_sigchld_event(0);
+
     g_delete_wait_obj(g_get_sync_event());
     g_set_sync_event(0);
 

xrdp/xrdp.h

@@ -55,6 +55,8 @@ g_set_sync1_mutex(long mutex);
 void
 g_set_term_event(tbus event);
 void
+g_set_sigchld_event(tbus event);
+void
 g_set_sync_event(tbus event);
 long
 g_get_threadid(void);
@@ -62,10 +64,14 @@ void
 g_set_threadid(long id);
 tbus
 g_get_term(void);
+tbus
+g_get_sigchld(void);
 int
 g_is_term(void);
 void
 g_set_term(int in_val);
+void
+g_set_sigchld(int in_val);
 tbus
 g_get_sync_event(void);
 void

xrdp/xrdp_listen.c

@@ -846,6 +846,29 @@ xrdp_listen_conn_in(struct trans *self, struct trans *new_self)
     return 0;
 }
 
+/*****************************************************************************/
+/**
+ * Process pending SIGCHLD events in the listen process
+ *
+ * The main reason for this is to log children which fail
+ * on a signal. This should be investigated.
+ */
+static void
+process_pending_sigchld_events(void)
+{
+    struct exit_status e;
+    int pid;
+
+    while ((pid = g_waitchild(&e)) > 0)
+    {
+        if (e.reason == E_XR_SIGNAL)
+        {
+            LOG(LOG_LEVEL_ERROR,
+                "Child %d terminated unexpectedly with signal \"%s\"",
+                pid, g_strsignal(e.val));
+        }
+    }
+}
 /*****************************************************************************/
 /* wait for incoming connections
    passes through trans_listen_address return value */
@@ -858,6 +881,7 @@ xrdp_listen_main_loop(struct xrdp_listen *self)
     int timeout;
     intptr_t robjs[32];
     intptr_t term_obj;
+    intptr_t sigchld_obj;
     intptr_t sync_obj;
     intptr_t done_obj;
     struct trans *ltrans;
@@ -876,6 +900,7 @@ xrdp_listen_main_loop(struct xrdp_listen *self)
         return 1;
     }
     term_obj = g_get_term(); /*Global termination event */
+    sigchld_obj = g_get_sigchld();
     sync_obj = g_get_sync_event();
     done_obj = self->pro_done_event;
     cont = 1;
@@ -884,6 +909,7 @@ xrdp_listen_main_loop(struct xrdp_listen *self)
         /* build the wait obj list */
         robjs_count = 0;
         robjs[robjs_count++] = term_obj;
+        robjs[robjs_count++] = sigchld_obj;
         robjs[robjs_count++] = sync_obj;
         robjs[robjs_count++] = done_obj;
         timeout = -1;
@@ -918,6 +944,12 @@ xrdp_listen_main_loop(struct xrdp_listen *self)
             break;
         }
 
+        if (g_is_wait_obj_set(sigchld_obj)) /* SIGCHLD caught */
+        {
+            g_set_sigchld(0);
+            process_pending_sigchld_events();
+        }
+
         /* some function must be processed by this thread */
         if (g_is_wait_obj_set(sync_obj))
         {

xrdp/xrdp_main_utils.c

@@ -39,6 +39,7 @@ static long g_threadid = 0; /* main threadid */
 static long g_sync_mutex = 0;
 static long g_sync1_mutex = 0;
 static tbus g_term_event = 0;
+static tbus g_sigchld_event = 0;
 static tbus g_sync_event = 0;
 /* synchronize stuff */
 static int g_sync_command = 0;
@@ -104,6 +105,19 @@ g_xrdp_sync(long (*sync_func)(long param1, long param2), long sync_param1,
     return sync_result;
 }
 
+/*****************************************************************************/
+/* Signal handler for SIGCHLD in the child
+ * Note: only signal safe code (eg. setting wait event) should be executed in
+ * this function. For more details see `man signal-safety`
+ */
+static void
+xrdp_child_sigchld_handler(int sig)
+{
+    while (g_waitchild(NULL) > 0)
+    {
+    }
+}
+
 /*****************************************************************************/
 /* called in child just after fork */
 int
@@ -112,12 +126,17 @@ xrdp_child_fork(void)
     int pid;
     char text[256];
 
-    /* close, don't delete these */
+    /* SIGCHLD in the child is of no interest to us */
+    g_signal_child_stop(xrdp_child_sigchld_handler);        /* SIGCHLD */
+
     g_close_wait_obj(g_term_event);
+    g_close_wait_obj(g_sigchld_event);
     g_close_wait_obj(g_sync_event);
+
     pid = g_getpid();
     g_snprintf(text, 255, "xrdp_%8.8x_main_term", pid);
     g_term_event = g_create_wait_obj(text);
+    g_sigchld_event = -1;
     g_snprintf(text, 255, "xrdp_%8.8x_main_sync", pid);
     g_sync_event = g_create_wait_obj(text);
     return 0;
@@ -158,6 +177,13 @@ g_set_term_event(tbus event)
     g_term_event = event;
 }
 
+/*****************************************************************************/
+void
+g_set_sigchld_event(tbus event)
+{
+    g_sigchld_event = event;
+}
+
 /*****************************************************************************/
 tbus
 g_get_sync_event(void)
@@ -193,6 +219,13 @@ g_get_term(void)
     return g_term_event;
 }
 
+/*****************************************************************************/
+tbus
+g_get_sigchld(void)
+{
+    return g_sigchld_event;
+}
+
 /*****************************************************************************/
 int
 g_is_term(void)
@@ -214,6 +247,20 @@ g_set_term(int in_val)
     }
 }
 
+/*****************************************************************************/
+void
+g_set_sigchld(int in_val)
+{
+    if (in_val)
+    {
+        g_set_wait_obj(g_sigchld_event);
+    }
+    else
+    {
+        g_reset_wait_obj(g_sigchld_event);
+    }
+}
+
 /*****************************************************************************/
 /*Some function must be called from the main thread.
  if g_sync_command==THREAD_WAITING a function is waiting to be processed*/