if child crashed (died of a signal) then log the signal if debug is on

Fri Jul 25 13:06:49 2003  George Lebl <jirka@5z.com>

	* daemon/display.c, daemon/gdm.c: if child crashed (died of a signal)
	  then log the signal if debug is on

	* daemon/gdm.h, daemon/server.c, daemon/slave.[ch]:  The X server
	  quite apparently emits a USR1 on reinit so catch that (apparently
	  I was not seeing this before as USR1 seems to have been blocked
	  during the fun times.)

	* demon/server.c: on exit from an XDMCP display whack all clients
	  to support broken X terminals

	* daemon/slave.c: whack the \n's from some of the GUI messages as
	  that's not needed anymore with gtk2 and in fact sometimes
	  can look like crap.

	* daemon/slave.c: run session_stop before the 10 second session
	  warning and don't run the warning if the server is dead

	* daemon/veriy-pam.c: don't return uninit value if we don't
	  open session or delete secred, return PAM_SUCCESS

	* daemon/gdmthemetester: add -terminate to command line so that
	  I don't keep getting thousands of Xnest windows on my screen

10 files changed, 294 insertions, 335 deletions

diff --git a/ChangeLog b/ChangeLog
index 3a7c8fa3..e6a40dfa 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,29 @@
+Fri Jul 25 13:06:49 2003  George Lebl <jirka@5z.com>
+
+	* daemon/display.c, daemon/gdm.c: if child crashed (died of a signal)
+	  then log the signal if debug is on
+
+	* daemon/gdm.h, daemon/server.c, daemon/slave.[ch]:  The X server
+	  quite apparently emits a USR1 on reinit so catch that (apparently
+	  I was not seeing this before as USR1 seems to have been blocked
+	  during the fun times.)
+
+	* demon/server.c: on exit from an XDMCP display whack all clients
+	  to support broken X terminals
+
+	* daemon/slave.c: whack the \n's from some of the GUI messages as
+	  that's not needed anymore with gtk2 and in fact sometimes
+	  can look like crap.
+
+	* daemon/slave.c: run session_stop before the 10 second session
+	  warning and don't run the warning if the server is dead
+
+	* daemon/veriy-pam.c: don't return uninit value if we don't
+	  open session or delete secred, return PAM_SUCCESS
+
+	* daemon/gdmthemetester: add -terminate to command line so that
+	  I don't keep getting thousands of Xnest windows on my screen
+
 Fri Jul 25 03:32:48 2003  George Lebl <jirka@5z.com>
 
 	* daemon/display.c: whack unneeded block pushes, when killing child
diff --git a/daemon/display.c b/daemon/display.c
index 607ff5c6..07866113 100644
--- a/daemon/display.c
+++ b/daemon/display.c
@@ -344,7 +344,8 @@ wait_again:
 	    }
 
 	    if (WIFSIGNALED (exitstatus)) {
-		    gdm_debug ("gdm_display_unmanage: Slave crashed, killing its children");
+		    gdm_debug ("gdm_display_unmanage: Slave crashed (signal %d), killing its children",
+			       (int)WTERMSIG (exitstatus));
 
 		    if (d->sesspid > 1)
 			    kill (-(d->sesspid), SIGTERM);
diff --git a/daemon/gdm.c b/daemon/gdm.c
index 4f021dc7..264d22a8 100644
--- a/daemon/gdm.c
+++ b/daemon/gdm.c
@@ -1117,14 +1117,17 @@ gdm_cleanup_children (void)
     if (WIFEXITED (exitstatus)) {
 	    status = WEXITSTATUS (exitstatus);
 	    crashed = FALSE;
+	    gdm_debug ("gdm_cleanup_children: child %d returned %d", pid, status);
     } else {
 	    status = EXIT_SUCCESS;
 	    crashed = TRUE;
+	    if (WIFSIGNALED (exitstatus))
+		    gdm_debug ("gdm_cleanup_children: child %d crashed of signal %d", pid,
+			       (int)WTERMSIG (exitstatus));
+	    else
+		    gdm_debug ("gdm_cleanup_children: child %d crashed", pid);
     }
 	
-    gdm_debug ("gdm_cleanup_children: child %d returned %d%s", pid, status,
-	       crashed ? " (child crashed)" : "");
-
     if (pid == extra_process) {
 	    /* an extra process died, yay! */
 	    extra_process = 0;
@@ -1312,6 +1315,7 @@ start_autopsy:
 	break;
 
     case DISPLAY_XFAILED:       /* X sucks */
+	gdm_debug ("X failed!");
 	/* inform about error if needed */
 	if (d->socket_conn != NULL) {
 		GdmConnection *conn = d->socket_conn;
@@ -1359,7 +1363,7 @@ start_autopsy:
 
     case DISPLAY_REMANAGE:	/* Remanage display */
     default:
-	gdm_debug ("gdm_child_action: Slave process returned %d", status);
+	gdm_debug ("gdm_child_action: In remanage");
 
 	/* inform about error if needed */
 	if (d->socket_conn != NULL) {
diff --git a/daemon/gdm.h b/daemon/gdm.h
index e77a1dba..9900327d 100644
--- a/daemon/gdm.h
+++ b/daemon/gdm.h
@@ -45,7 +45,7 @@
  * process exit codes */
 #define SERVER_TIMEOUT 2	/* Server didn't start */
 #define SERVER_DEAD 250		/* Server stopped */
-#define SERVER_STARTED 251	/* Server started but not ready for connections yet */
+#define SERVER_PENDING 251	/* Server started but not ready for connections yet */
 #define SERVER_RUNNING 252	/* Server running and ready for connections */
 #define SERVER_ABORT 253	/* Server failed badly. Suspending display. */
 
diff --git a/daemon/server.c b/daemon/server.c
index a5218743..7f337e56 100644
--- a/daemon/server.c
+++ b/daemon/server.c
@@ -54,7 +54,6 @@
 /* Local prototypes */
 static void gdm_server_spawn (GdmDisplay *d, const char *vtarg);
 static void gdm_server_usr1_handler (gint);
-static void gdm_server_alarm_handler (gint);
 static void gdm_server_child_handler (gint);
 static char * get_font_path (const char *display);
 
@@ -75,6 +74,9 @@ static GdmDisplay *d = NULL;
 static gboolean server_signal_notified = FALSE;
 static int server_signal_pipe[2];
 
+static void do_server_wait (GdmDisplay *d);
+static gboolean setup_server_wait (GdmDisplay *d);
+
 void
 gdm_server_whack_lockfile (GdmDisplay *disp)
 {
@@ -140,14 +142,11 @@ jumpback_xioerror_handler (Display *disp)
  * but only if the display exists
  */
 
-void
+gboolean
 gdm_server_reinit (GdmDisplay *disp)
 {
-	int (*old_xerror_handler)(Display *, XErrorEvent *) = NULL;
-	int (*old_xioerror_handler)(Display *) = NULL;
-
 	if (disp == NULL)
-		return;
+		return FALSE;
 
 	if (disp->servpid <= 0) {
 		/* Kill our connection if one existed, likely to result
@@ -156,64 +155,66 @@ gdm_server_reinit (GdmDisplay *disp)
 			XCloseDisplay (disp->dsp);
 			disp->dsp = NULL;
 		}
-		return;
+		return FALSE;
 	}
 
 	gdm_debug ("gdm_server_reinit: Server for %s is about to be reinitialized!", disp->name);
 
-	/* FIXME: the X server emits a SIGUSR1 so whack the below hack and add a server
-	   wait thingie like during start */
+	if ( ! setup_server_wait (disp))
+		return FALSE;
 
-	/* Long live Setjmp, DIE DIE DIE XSetIOErrorHandler */
+	d->servstat = SERVER_PENDING;
 
-	if (disp->dsp == NULL) {
-		gdm_error ("Reiniting server, but don't have a display connection, strange");
+	if (disp->dsp != NULL) {
+		int (*old_xerror_handler)(Display *, XErrorEvent *) = NULL;
+		int (*old_xioerror_handler)(Display *) = NULL;
 
-		/* Now whack the server with a SIGHUP */
-		gdm_sigchld_block_push ();
-		if (disp->servpid > 1)
-			kill (disp->servpid, SIGHUP);
-		gdm_sigchld_block_pop ();
-		/* a hack we have no way of knowing when the server died */
-		sleep (1);
-		return;
-	}
+		/* Do note the interaction of this Setjmp and the signal
+	   	   handlers and the Setjmp in slave.c */
 
-	/* Do note the interaction of this Setjmp and the signal
-	   handlers and the Setjmp in slave.c */
+		/* Long live Setjmp, DIE DIE DIE XSetIOErrorHandler */
 
-	if (Setjmp (reinitjmp) == 0)  {
-		/* come here and we'll whack the server and wait to get
-		   an xio error */
-		old_xerror_handler = XSetErrorHandler (ignore_xerror_handler);
-		old_xioerror_handler = XSetIOErrorHandler (jumpback_xioerror_handler);
+		if (Setjmp (reinitjmp) == 0)  {
+			/* come here and we'll whack the server and wait to get
+			   an xio error */
+			old_xerror_handler = XSetErrorHandler (ignore_xerror_handler);
+			old_xioerror_handler = XSetIOErrorHandler (jumpback_xioerror_handler);
 
-		/* Now whack the server with a SIGHUP */
-		gdm_sigchld_block_push ();
-		if (disp->servpid > 1) {
-			kill (disp->servpid, SIGHUP);
-		} else {
+			/* Now whack the server with a SIGHUP */
+			gdm_sigchld_block_push ();
+			if (disp->servpid > 1)
+				kill (disp->servpid, SIGHUP);
+			else
+				d->servstat = SERVER_DEAD;
 			gdm_sigchld_block_pop ();
+
 			/* the server is dead, weird */
 			if (disp->dsp != NULL) {
 				XCloseDisplay (disp->dsp);
 				disp->dsp = NULL;
 			}
-			sleep (1);
-			Longjmp (reinitjmp, 1);
 		}
+		/* no more display */
+		disp->dsp = NULL;
+		XSetErrorHandler (old_xerror_handler);
+		XSetIOErrorHandler (old_xioerror_handler);
+	} else {
+		/* Now whack the server with a SIGHUP */
+		gdm_sigchld_block_push ();
+		if (disp->servpid > 1)
+			kill (disp->servpid, SIGHUP);
+		else
+			d->servstat = SERVER_DEAD;
 		gdm_sigchld_block_pop ();
-
-		/* Wait for an xioerror */
-		for (;;) {
-			XEvent event;
-			XNextEvent (disp->dsp, &event);
-		}
 	}
-	/* no more display */
-	disp->dsp = NULL;
-	XSetErrorHandler (old_xerror_handler);
-	XSetIOErrorHandler (old_xioerror_handler);
+
+	/* Wait for the SIGUSR1 */
+	do_server_wait (d);
+
+	if (d->servstat == SERVER_RUNNING)
+		return TRUE;
+	else
+		return FALSE;
 }
 
 /**
@@ -233,6 +234,9 @@ gdm_server_stop (GdmDisplay *disp)
 
     /* Kill our connection if one existed */
     if (disp->dsp != NULL) {
+	    /* on XDMCP servers first kill everything in sight */
+	    if (disp->type == TYPE_XDMCP)
+		    gdm_server_whack_clients (d);
 	    XCloseDisplay (disp->dsp);
 	    disp->dsp = NULL;
     }
@@ -434,103 +438,25 @@ display_vt (GdmDisplay *disp)
 	return -1;
 }
 
-static void
-check_child_status (void)
-{
-	int status;
-	pid_t pid;
-
-	while ((pid = waitpid (-1, &status, WNOHANG)) > 0) {
-		gdm_debug ("check_child_status: %d died", pid);
-
-		if (WIFEXITED (status))
-			gdm_debug ("check_child_status: %d returned %d",
-				   (int)pid, (int)WEXITSTATUS (status));
-		if (WIFSIGNALED (status))
-			gdm_debug ("check_child_status: %d died of %d",
-				   (int)pid, (int)WTERMSIG (status));
-
-		if (pid == d->servpid) {
-			gdm_debug ("check_child_status: Got SIGCHLD from server, "
-				   "server abort");
-
-			d->servstat = SERVER_ABORT;	/* Server died unexpectedly */
-			d->servpid = 0;
-
-			server_signal_notified = TRUE;
-		} else if (pid == extra_process) {
-			/* an extra process died, yay! */
-			extra_process = 0;
-			extra_status = status;
-		}
-	}
-}
-
-/**
- * gdm_server_start:
- * @disp: Pointer to a GdmDisplay structure
- *
- * Starts a local X server. Handles retries and fatal errors properly.
- */
+static struct sigaction old_svr_wait_chld;
+static sigset_t old_svr_wait_mask;
 
-gboolean
-gdm_server_start (GdmDisplay *disp, gboolean treat_as_flexi,
-		  int min_flexi_disp, int flexi_retries)
+static gboolean
+setup_server_wait (GdmDisplay *d)
 {
-    struct sigaction usr1, chld, alrm;
-    struct sigaction old_chld, old_alrm;
-    sigset_t mask, oldmask;
-    int flexi_disp = 20;
-    char *vtarg = NULL;
-    int vtfd = -1, vt = -1;
-    
-    if (disp == NULL)
-	    return FALSE;
-
-    d = disp;
-
-    /* if an X server exists, wipe it */
-    gdm_server_stop (d);
-
-    /* First clear the VT number */
-    if (d->type == TYPE_LOCAL ||
-	d->type == TYPE_FLEXI) {
-	    d->vt = -1;
-	    gdm_slave_send_num (GDM_SOP_VT_NUM, -1);
-    }
-
-    if (SERVER_IS_FLEXI (d) ||
-	treat_as_flexi) {
-	    flexi_disp = gdm_get_free_display
-		    (MAX (high_display_num + 1, min_flexi_disp) /* start */,
-		     d->server_uid /* server uid */);
-
-	    g_free (d->name);
-	    d->name = g_strdup_printf (":%d", flexi_disp);
-	    d->dispnum = flexi_disp;
-
-	    gdm_slave_send_num (GDM_SOP_DISP_NUM, flexi_disp);
-    }
-
-
-    gdm_debug ("gdm_server_start: %s", d->name);
-
-    /* Create new cookie */
-    if ( ! gdm_auth_secure_display (d)) 
-	    return FALSE;
-    gdm_slave_send_string (GDM_SOP_COOKIE, d->cookie);
-    ve_setenv ("DISPLAY", d->name, TRUE);
+    struct sigaction usr1, chld;
+    sigset_t mask;
 
     if (pipe (server_signal_pipe) != 0) {
 	    gdm_error (_("%s: Error opening a pipe: %s"),
-		       "gdm_server_start", strerror (errno));
+		       "setup_server_wait", strerror (errno));
 	    return FALSE; 
     }
     server_signal_notified = FALSE;
 
     /* Catch USR1 from X server */
     usr1.sa_handler = gdm_server_usr1_handler;
-    usr1.sa_flags = SA_RESTART|SA_RESETHAND;
+    usr1.sa_flags = SA_RESTART;
     sigemptyset (&usr1.sa_mask);
 
     if (sigaction (SIGUSR1, &usr1, NULL) < 0) {
@@ -543,10 +469,10 @@ gdm_server_start (GdmDisplay *disp, gboolean treat_as_flexi,
 
     /* Catch CHLD from X server */
     chld.sa_handler = gdm_server_child_handler;
-    chld.sa_flags = SA_RESTART|SA_RESETHAND;
+    chld.sa_flags = SA_RESTART|SA_NOCLDSTOP;
     sigemptyset (&chld.sa_mask);
 
-    if (sigaction (SIGCHLD, &chld, &old_chld) < 0) {
+    if (sigaction (SIGCHLD, &chld, &old_svr_wait_chld) < 0) {
 	    gdm_error (_("%s: Error setting up CHLD signal handler: %s"),
 		       "gdm_server_start", strerror (errno));
 	    signal (SIGUSR1, SIG_IGN);
@@ -555,73 +481,37 @@ gdm_server_start (GdmDisplay *disp, gboolean treat_as_flexi,
 	    return FALSE;
     }
 
-    /* Catch ALRM from X server */
-    alrm.sa_handler = gdm_server_alarm_handler;
-    alrm.sa_flags = SA_RESTART|SA_RESETHAND;
-    sigemptyset (&alrm.sa_mask);
-
-    if (sigaction (SIGALRM, &alrm, &old_alrm) < 0) {
-	    gdm_error (_("%s: Error setting up ALRM signal handler: %s"),
-		       "gdm_server_start", strerror (errno));
-	    signal (SIGUSR1, SIG_IGN);
-	    sigaction (SIGCHLD, &old_chld, NULL);
-	    close (server_signal_pipe[0]);
-	    close (server_signal_pipe[1]);
-	    return FALSE;
-    }
-
     /* Set signal mask */
     sigemptyset (&mask);
     sigaddset (&mask, SIGUSR1);
     sigaddset (&mask, SIGCHLD);
-    sigaddset (&mask, SIGALRM);
-    sigprocmask (SIG_UNBLOCK, &mask, &oldmask);
-
-    /* We add a timeout in case the X server fails to start. This
-     * might happen because X servers take a while to die, close their
-     * sockets etc. If the old X server isn't completely dead, the new
-     * one will fail and we'll hang here forever */
-
-    /* Only do alarm if server will be run as root */
-    if (d->server_uid == 0) {
-	    alarm (SERVER_WAIT_ALARM);
-    }
-
-    d->servstat = SERVER_DEAD;
-
-    if (d->type == TYPE_LOCAL ||
-	d->type == TYPE_FLEXI) {
-	    vtarg = gdm_get_empty_vt_argument (&vtfd, &vt);
-    }
+    sigprocmask (SIG_UNBLOCK, &mask, &old_svr_wait_mask);
 
-    /* fork X server process */
-    gdm_server_spawn (d, vtarg);
-
-    /* we can now use d->handled since that's set up above */
+    return TRUE;
+}
 
+static void
+do_server_wait (GdmDisplay *d)
+{
     /* Wait for X server to send ready signal */
-    if (d->servstat == SERVER_STARTED) {
+    if (d->servstat == SERVER_PENDING) {
 	    if (d->server_uid != 0 && ! d->handled) {
-		    alarm (0);
 		    /* FIXME: If not handled, we just don't know, so
 		     * just wait a few seconds and hope things just work,
 		     * fortunately there is no such case yet and probably
 		     * never will, but just for code anality's sake */
 		    sleep (5);
-		    /* In case we got a SIGCHLD */
-		    check_child_status ();
 	    } else if (d->server_uid != 0) {
 		    int i;
 
-		    alarm (0);
+		    /* FIXME: This is not likely to work in reinit,
+		       but we never reinit Xnest servers nowdays,
+		       so that's fine */
 
 		    /* if we're running the server as a non-root, we can't
 		     * use USR1 of course, so try openning the display 
 		     * as a test, but the */
 
-		    /* In case we got a SIGCHLD */
-		    check_child_status ();
-
 		    /* just in case it's set */
 		    ve_unsetenv ("XAUTHORITY");
 
@@ -629,7 +519,7 @@ gdm_server_start (GdmDisplay *disp, gboolean treat_as_flexi,
 
 		    for (i = 0;
 			 d->dsp == NULL &&
-			 d->servstat == SERVER_STARTED &&
+			 d->servstat == SERVER_PENDING &&
 			 i < SERVER_WAIT_ALARM;
 			 i++) {
 			    d->dsp = XOpenDisplay (d->name);
@@ -637,46 +527,130 @@ gdm_server_start (GdmDisplay *disp, gboolean treat_as_flexi,
 				    sleep (1);
 			    else
 				    d->servstat = SERVER_RUNNING;
-
-			    /* In case we got a SIGCHLD */
-			    check_child_status ();
 		    }
 		    if (d->dsp == NULL &&
 			/* Note: we could have still gotten a SIGCHLD */
-			d->servstat == SERVER_STARTED) {
+			d->servstat == SERVER_PENDING) {
 			    d->servstat = SERVER_TIMEOUT;
 		    }
+		    if (d->servpid <= 1) {
+			    d->servstat = SERVER_ABORT;
+		    }
 	    } else {
+		    time_t t = time (NULL);
 
-		    gdm_debug ("gdm_server_start: Before mainloop waiting for server");
+		    gdm_debug ("do_server_wait: Before mainloop waiting for server");
 
 		    do {
 			    fd_set rfds;
+			    struct timeval tv;
+
+			    /* Wait up to SERVER_WAIT_ALARM seconds. */
+			    tv.tv_sec = MAX (1, SERVER_WAIT_ALARM - (time(NULL) - t));
+			    tv.tv_usec = 0;
 
 			    FD_ZERO (&rfds);
 			    FD_SET (server_signal_pipe[0], &rfds);
 
-			    if (select (server_signal_pipe[0]+1, &rfds, NULL, NULL, NULL) > 0) {
+			    if (select (server_signal_pipe[0]+1, &rfds, NULL, NULL, &tv) > 0) {
 				    char buf[4];
 				    /* read the Yay! */
 				    read (server_signal_pipe[0], buf, 4);
 			    }
-			    /* In case we got a SIGCHLD */
-			    check_child_status ();
+			    if ( ! server_signal_notified &&
+				t + SERVER_WAIT_ALARM < time (NULL)) {
+				    gdm_debug ("do_server_wait: Server timeout");
+				    d->servstat = SERVER_TIMEOUT;
+				    server_signal_notified = TRUE;
+			    }
+			    if (d->servpid <= 1) {
+				    d->servstat = SERVER_ABORT;
+				    server_signal_notified = TRUE;
+			    }
 		    } while ( ! server_signal_notified);
 
 		    gdm_debug ("gdm_server_start: After mainloop waiting for server");
 	    }
     }
 
-    /* In case we got a SIGCHLD */
-    check_child_status ();
+    /* restore default handlers */
+    signal (SIGUSR1, SIG_IGN);
+    sigaction (SIGCHLD, &old_svr_wait_chld, NULL);
+    sigprocmask (SIG_SETMASK, &old_svr_wait_mask, NULL);
+
+    close (server_signal_pipe[0]);
+    close (server_signal_pipe[1]);
+}
+
+/**
+ * gdm_server_start:
+ * @disp: Pointer to a GdmDisplay structure
+ *
+ * Starts a local X server. Handles retries and fatal errors properly.
+ */
+
+gboolean
+gdm_server_start (GdmDisplay *disp, gboolean treat_as_flexi,
+		  int min_flexi_disp, int flexi_retries)
+{
+    int flexi_disp = 20;
+    char *vtarg = NULL;
+    int vtfd = -1, vt = -1;
+    
+    if (disp == NULL)
+	    return FALSE;
+
+    d = disp;
+
+    /* if an X server exists, wipe it */
+    gdm_server_stop (d);
+
+    /* First clear the VT number */
+    if (d->type == TYPE_LOCAL ||
+	d->type == TYPE_FLEXI) {
+	    d->vt = -1;
+	    gdm_slave_send_num (GDM_SOP_VT_NUM, -1);
+    }
+
+    if (SERVER_IS_FLEXI (d) ||
+	treat_as_flexi) {
+	    flexi_disp = gdm_get_free_display
+		    (MAX (high_display_num + 1, min_flexi_disp) /* start */,
+		     d->server_uid /* server uid */);
+
+	    g_free (d->name);
+	    d->name = g_strdup_printf (":%d", flexi_disp);
+	    d->dispnum = flexi_disp;
+
+	    gdm_slave_send_num (GDM_SOP_DISP_NUM, flexi_disp);
+    }
+
+
+    gdm_debug ("gdm_server_start: %s", d->name);
+
+    /* Create new cookie */
+    if ( ! gdm_auth_secure_display (d)) 
+	    return FALSE;
+    gdm_slave_send_string (GDM_SOP_COOKIE, d->cookie);
+    ve_setenv ("DISPLAY", d->name, TRUE);
+
+    if ( ! setup_server_wait (d))
+	    return FALSE;
+
+    d->servstat = SERVER_DEAD;
 
-    /* Unset alarm */
-    if (d->server_uid == 0) {
-	    alarm (0);
+    if (d->type == TYPE_LOCAL ||
+	d->type == TYPE_FLEXI) {
+	    vtarg = gdm_get_empty_vt_argument (&vtfd, &vt);
     }
 
+    /* fork X server process */
+    gdm_server_spawn (d, vtarg);
+
+    /* we can now use d->handled since that's set up above */
+
+    do_server_wait (d);
+
     /* If we were holding a vt open for the server, close it now as it has
      * already taken the bait. */
     if (vtfd > 0)
@@ -695,16 +669,6 @@ gdm_server_start (GdmDisplay *disp, gboolean treat_as_flexi,
     case SERVER_RUNNING:
 	    gdm_debug ("gdm_server_start: Completed %s!", d->name);
 
-	    sigprocmask (SIG_SETMASK, &oldmask, NULL);
-
-	    /* restore default handlers */
-	    signal (SIGUSR1, SIG_IGN);
-	    sigaction (SIGCHLD, &old_chld, NULL);
-	    sigaction (SIGALRM, &old_alrm, NULL);
-
-	    close (server_signal_pipe[0]);
-	    close (server_signal_pipe[1]);
-
 	    if (SERVER_IS_FLEXI (d))
 		    gdm_slave_send_num (GDM_SOP_FLEXI_OK, 0 /* bogus */);
 	    if (d->type == TYPE_LOCAL ||
@@ -743,19 +707,6 @@ gdm_server_start (GdmDisplay *disp, gboolean treat_as_flexi,
     /* We will rebake cookies anyway, so wipe these */
     gdm_server_wipe_cookies (disp);
 
-    sigprocmask (SIG_SETMASK, &oldmask, NULL);
-
-    /* In case we got a SIGCHLD */
-    check_child_status ();
-
-    /* restore default handlers */
-    signal (SIGUSR1, SIG_IGN);
-    sigaction (SIGCHLD, &old_chld, NULL);
-    sigaction (SIGALRM, &old_alrm, NULL);
-
-    close (server_signal_pipe[0]);
-    close (server_signal_pipe[1]);
-
     if (disp->type == TYPE_FLEXI_XNEST &&
 	display_xnest_no_connect (disp)) {
 	    gdm_slave_send_num (GDM_SOP_FLEXI_ERR,
@@ -989,7 +940,7 @@ gdm_server_resolve_command_line (GdmDisplay *disp,
 static void 
 gdm_server_spawn (GdmDisplay *d, const char *vtarg)
 {
-    struct sigaction ign_signal, dfl_signal;
+    struct sigaction ign_signal;
     sigset_t mask;
     gchar **argv = NULL;
     char *logfile;
@@ -1002,7 +953,7 @@ gdm_server_spawn (GdmDisplay *d, const char *vtarg)
 	    return;
     }
 
-    d->servstat = SERVER_STARTED;
+    d->servstat = SERVER_PENDING;
 
     gdm_sigchld_block_push ();
 
@@ -1035,11 +986,8 @@ gdm_server_spawn (GdmDisplay *d, const char *vtarg)
     switch (pid) {
 	
     case 0:
-        alarm (0);
-
-	/* Close the XDMCP fd inherited by the daemon process */
-	if (GdmXdmcp)
-		gdm_xdmcp_close();
+	/* the pops whacked mask again */
+        gdm_unset_signals ();
 
 	closelog ();
 
@@ -1076,10 +1024,13 @@ gdm_server_spawn (GdmDisplay *d, const char *vtarg)
 	ign_signal.sa_flags = SA_RESTART;
 	sigemptyset (&ign_signal.sa_mask);
 
-	if (sigaction (SIGUSR1, &ign_signal, NULL) < 0) {
-		gdm_error (_("%s: Error setting %s to %s"),
-			   "gdm_server_spawn", "USR1", "SIG_IGN");
-		_exit (SERVER_ABORT);
+	if (d->server_uid == 0) {
+		/* only set this if we can actually listen */
+		if (sigaction (SIGUSR1, &ign_signal, NULL) < 0) {
+			gdm_error (_("%s: Error setting %s to %s"),
+				   "gdm_server_spawn", "USR1", "SIG_IGN");
+			_exit (SERVER_ABORT);
+		}
 	}
 	if (sigaction (SIGTTIN, &ign_signal, NULL) < 0) {
 		gdm_error (_("%s: Error setting %s to %s"),
@@ -1092,21 +1043,8 @@ gdm_server_spawn (GdmDisplay *d, const char *vtarg)
 		_exit (SERVER_ABORT);
 	}
 
-	/* And HUP and TERM should be at default */
-	dfl_signal.sa_handler = SIG_DFL;
-	dfl_signal.sa_flags = SA_RESTART;
-	sigemptyset (&dfl_signal.sa_mask);
-
-	if (sigaction (SIGHUP, &dfl_signal, NULL) < 0) {
-		gdm_error (_("%s: Error setting %s to %s"),
-			   "gdm_server_spawn", "HUP", "SIG_DFL");
-		_exit (SERVER_ABORT);
-	}
-	if (sigaction (SIGTERM, &dfl_signal, NULL) < 0) {
-		gdm_error (_("%s: Error setting %s to %s"),
-			   "gdm_server_spawn", "TERM", "SIG_DFL");
-		_exit (SERVER_ABORT);
-	}
+	/* And HUP and TERM are at SIG_DFL from gdm_unset_signals,
+	   we also have an empty mask and all that fun stuff */
 
 	/* unblock signals (especially HUP/TERM/USR1) so that we
 	 * can control the X server */
@@ -1232,8 +1170,10 @@ gdm_server_usr1_handler (gint sig)
 {
     gdm_in_signal++;
 
-    d->servstat = SERVER_RUNNING; /* Server ready to accept connections */
-    d->starttime = time (NULL);
+    if (d->servpid > 1) {
+	    d->servstat = SERVER_RUNNING; /* Server ready to accept connections */
+	    d->starttime = time (NULL);
+    }
 
     gdm_debug ("gdm_server_usr1_handler: Got SIGUSR1, server running");
 
@@ -1246,30 +1186,6 @@ gdm_server_usr1_handler (gint sig)
 
 
 /**
- * gdm_server_alarm_handler:
- * @sig: Signal value
- *
- * Server start timeout handler
- */
-
-static void 
-gdm_server_alarm_handler (gint signal)
-{
-    gdm_in_signal++;
-
-    d->servstat = SERVER_TIMEOUT; /* Server didn't start */
-
-    gdm_debug ("gdm_server_alarm_handler: Got SIGALRM, server abort");
-
-    server_signal_notified = TRUE;
-    /* this will quit the select */
-    write (server_signal_pipe[1], "Yay!", 4);
-
-    gdm_in_signal--;
-}
-
-
-/**
  * gdm_server_child_handler:
  * @sig: Signal value
  *
@@ -1283,6 +1199,9 @@ gdm_server_child_handler (int signal)
 
 	gdm_debug ("gdm_server_child_handler: Got SIGCHLD");
 
+	/* go to the main child handler */
+	gdm_slave_child_handler (signal);
+
 	/* this will quit the select */
 	write (server_signal_pipe[1], "Yay!", 4);
 
diff --git a/daemon/server.h b/daemon/server.h
index 2ad60d3f..ddf6f8a8 100644
--- a/daemon/server.h
+++ b/daemon/server.h
@@ -32,7 +32,7 @@ gboolean	gdm_server_start	(GdmDisplay *d,
 					 int min_flexi_disp,
 					 int flexi_retries);
 void		gdm_server_stop		(GdmDisplay *d);
-void		gdm_server_reinit	(GdmDisplay *d);
+gboolean	gdm_server_reinit	(GdmDisplay *d);
 GdmDisplay *	gdm_server_alloc	(gint id,
 					 const gchar *command);
 void		gdm_server_whack_clients (GdmDisplay *disp);
diff --git a/daemon/slave.c b/daemon/slave.c
index 11ccbf24..895683d2 100644
--- a/daemon/slave.c
+++ b/daemon/slave.c
@@ -176,7 +176,6 @@ static void     gdm_slave_session_stop (gboolean run_post_session,
 					gboolean no_shutdown_check);
 static void     gdm_slave_alrm_handler (int sig);
 static void     gdm_slave_term_handler (int sig);
-static void     gdm_slave_child_handler (int sig);
 static void     gdm_slave_usr2_handler (int sig);
 static void     gdm_slave_quick_exit (gint status);
 static void     gdm_slave_exit (gint status, const gchar *format, ...) G_GNUC_PRINTF (2, 3);
@@ -556,6 +555,7 @@ gdm_slave_start (GdmDisplay *display)
 	sigdelset (&mask, SIGTERM);
 	sigdelset (&mask, SIGCHLD);
 	sigdelset (&mask, SIGUSR2);
+	sigdelset (&mask, SIGUSR1); /* normally we ignore USR1 */
 	if (display->type == TYPE_XDMCP &&
 	    GdmPingInterval > 0) {
 		sigdelset (&mask, SIGALRM);
@@ -607,7 +607,10 @@ gdm_slave_start (GdmDisplay *display)
 			}
 			gdm_slave_send_string (GDM_SOP_COOKIE, d->cookie);
 
-			gdm_server_reinit (d);
+			if ( ! gdm_server_reinit (d)) {
+				gdm_error ("Error reinitilizing server");
+				gdm_slave_quick_exit (DISPLAY_REMANAGE);
+			}
 		}
 	}
 	/* very very very evil, should never break, we can't return from
@@ -1344,10 +1347,10 @@ run_config (GdmDisplay *display, struct passwd *pwent)
 
 		gdm_error_box (d,
 			       GTK_MESSAGE_ERROR,
-			       _("Could not execute the configuration\n"
-				 "program.  Make sure it's path is set\n"
-				 "correctly in the configuration file.\n"
-				 "I will attempt to start it from the\n"
+			       _("Could not execute the configuration "
+				 "program.  Make sure it's path is set "
+				 "correctly in the configuration file.  "
+				 "I will attempt to start it from the "
 				 "default location."));
 
 		argv = ve_split
@@ -1358,8 +1361,8 @@ run_config (GdmDisplay *display, struct passwd *pwent)
 
 		gdm_error_box (d,
 			       GTK_MESSAGE_ERROR,
-			       _("Could not execute the configuration\n"
-				 "program.  Make sure it's path is set\n"
+			       _("Could not execute the configuration "
+				 "program.  Make sure it's path is set "
 				 "correctly in the configuration file."));
 
 		_exit (0);
@@ -2118,13 +2121,13 @@ gdm_slave_greeter (void)
 	if(gdm_emergency_server) {
 		gdm_error_box (d,
 			       GTK_MESSAGE_ERROR,
-			       _("No servers were defined in the\n"
-				 "configuration file and XDMCP was\n"
-				 "disabled.  This can only be a\n"
-				 "configuration error.  So I have started\n"
-				 "a single server for you.  You should\n"
-				 "log in and fix the configuration.\n"
-				 "Note that automatic and timed logins\n"
+			       _("No servers were defined in the "
+				 "configuration file and XDMCP was "
+				 "disabled.  This can only be a "
+				 "configuration error.  So I have started "
+				 "a single server for you.  You should "
+				 "log in and fix the configuration.  "
+				 "Note that automatic and timed logins "
 				 "are disabled now."));
 		ve_unsetenv ("GDM_TIMED_LOGIN_OK");
 	}
@@ -2132,10 +2135,10 @@ gdm_slave_greeter (void)
 	if (d->failsafe_xserver) {
 		gdm_error_box (d,
 			       GTK_MESSAGE_ERROR,
-			       _("I could not start the regular X\n"
-				 "server (your graphical environment)\n"
-				 "and so this is a failsafe X server.\n"
-				 "You should log in and properly\n"
+			       _("I could not start the regular X "
+				 "server (your graphical environment) "
+				 "and so this is a failsafe X server.  "
+				 "You should log in and properly "
 				 "configure the X server."));
 	}
 
@@ -2206,10 +2209,10 @@ gdm_slave_greeter (void)
 
 	gdm_error_box (d,
 		       GTK_MESSAGE_ERROR,
-		       _("Cannot start the greeter program,\n"
-			 "you will not be able to log in.\n"
-			 "This display will be disabled.\n"
-			 "Try logging in by other means and\n"
+		       _("Cannot start the greeter program, "
+			 "you will not be able to log in.  "
+			 "This display will be disabled.  "
+			 "Try logging in by other means and "
 			 "editing the configuration file"));
 	
 	/* If no greeter we really have to disable the display */
@@ -2562,9 +2565,9 @@ gdm_slave_chooser (void)
 
 		gdm_error_box (d,
 			       GTK_MESSAGE_ERROR,
-			       _("Cannot start the chooser program,\n"
-				 "you will probably not be able to log in.\n"
-				 "Please contact the system administrator.\n"));
+			       _("Cannot start the chooser program, "
+				 "you will probably not be able to log in.  "
+				 "Please contact the system administrator."));
 
 		gdm_child_exit (DISPLAY_REMANAGE, _("gdm_slave_chooser: Error starting chooser on display %s"), d->name);
 
@@ -2976,18 +2979,18 @@ session_child_run (struct passwd *pwent,
 			session = GDM_SESSION_FAILSAFE_XTERM;
 			gdm_error_box
 				(d, GTK_MESSAGE_ERROR,
-				 _("Could not find the GNOME installation,\n"
-				   "will try running the \"Failsafe xterm\"\n"
+				 _("Could not find the GNOME installation, "
+				   "will try running the \"Failsafe xterm\" "
 				   "session."));
 		} else {
 			argv[1] = "--failsafe";
 			argv[2] = NULL;
 			gdm_error_box
 				(d, GTK_MESSAGE_INFO,
-				 _("This is the Failsafe Gnome session.\n"
-				   "You will be logged into the 'Default'\n"
-				   "session of Gnome with no startup scripts\n"
-				   "run.  This is only to fix problems in\n"
+				 _("This is the Failsafe Gnome session.  "
+				   "You will be logged into the 'Default' "
+				   "session of Gnome with no startup scripts "
+				   "run.  This is only to fix problems in "
 				   "your installation."));
 		}
 		failsafe = TRUE;
@@ -3011,11 +3014,11 @@ session_child_run (struct passwd *pwent,
 			argv[3] = NULL;
 			gdm_error_box
 				(d, GTK_MESSAGE_INFO,
-				 _("This is the Failsafe xterm session.\n"
-				   "You will be logged into a terminal\n"
-				   "console so that you may fix your system\n"
-				   "if you cannot log in any other way.\n"
-				   "To exit the terminal emulator, type\n"
+				 _("This is the Failsafe xterm session.  "
+				   "You will be logged into a terminal "
+				   "console so that you may fix your system "
+				   "if you cannot log in any other way.  "
+				   "To exit the terminal emulator, type "
 				   "'exit' and an enter into the window."));
 			focus_first_x_window ("xterm");
 		}
@@ -3041,7 +3044,7 @@ session_child_run (struct passwd *pwent,
 		gdm_error (_("%s: User not allowed to log in"),
 			   "gdm_slave_session_start");
 		gdm_error_box (d, GTK_MESSAGE_ERROR,
-			       _("The system administrator has\n"
+			       _("The system administrator has "
 				 "disabled your account."));
 		/* ends as if nothing bad happened */
 		_exit (0);
@@ -3119,10 +3122,10 @@ gdm_slave_session_start (void)
 	! g_file_test (pwent->pw_dir, G_FILE_TEST_IS_DIR)) {
 	    char *msg = g_strdup_printf (
 		     _("Your home directory is listed as:\n'%s'\n"
-		       "but it does not appear to exist.\n"
-		       "Do you want to log in with the / (root)\n"
+		       "but it does not appear to exist.  "
+		       "Do you want to log in with the / (root) "
 		       "directory as your home directory?\n\n"
-		       "It is unlikely anything will work unless\n"
+		       "It is unlikely anything will work unless "
 		       "you use a failsafe session."),
 		     ve_sure_string (pwent->pw_dir));
 
@@ -3254,11 +3257,11 @@ gdm_slave_session_start (void)
 
 	    gdm_error_box (d,
 			   GTK_MESSAGE_ERROR,
-			   _("GDM could not write to your authorization\n"
-			     "file.  This could mean that you are out of\n"
-			     "disk space or that your home directory could\n"
-			     "not be opened for writing.  In any case, it\n"
-			     "is not possible to log in.  Please contact\n"
+			   _("GDM could not write to your authorization "
+			     "file.  This could mean that you are out of "
+			     "disk space or that your home directory could "
+			     "not be opened for writing.  In any case, it "
+			     "is not possible to log in.  Please contact "
 			     "your system administrator"));
 
 	    gdm_slave_session_stop (FALSE /* run_post_session */,
@@ -3371,8 +3374,13 @@ gdm_slave_session_start (void)
     gdm_debug ("Session: start_time: %ld end_time: %ld",
 	       (long)session_start_time, (long)end_time);
 
+    gdm_slave_session_stop (pid != 0 /* run_post_session */,
+			    FALSE /* no_shutdown_check */);
+
     if  ((/* sanity */ end_time >= session_start_time) &&
-	 (end_time - 10 <= session_start_time)) {
+	 (end_time - 10 <= session_start_time) &&
+	 /* only if the X server still exist! */
+	 d->servpid > 1) {
 	    char *errfile = g_build_filename (home_dir, ".xsession-errors", NULL);
 	    gdm_debug ("Session less than 10 seconds!");
 
@@ -3380,12 +3388,12 @@ gdm_slave_session_start (void)
 	     * such as gnome-session missing and such things. */
 	    gdm_error_box_full (d,
 				GTK_MESSAGE_WARNING,
-				_("Your session only lasted less than\n"
-				  "10 seconds.  If you have not logged out\n"
-				  "yourself, this could mean that there is\n"
-				  "some installation problem or that you may\n"
-				  "be out of diskspace.  Try logging in with\n"
-				  "one of the failsafe sessions to see if you\n"
+				_("Your session only lasted less than "
+				  "10 seconds.  If you have not logged out "
+				  "yourself, this could mean that there is "
+				  "some installation problem or that you may "
+				  "be out of diskspace.  Try logging in with "
+				  "one of the failsafe sessions to see if you "
 				  "can fix this problem."),
 				(home_dir_ok && ! failsafe) ?
 			       	  _("View details (~/.xsession-errors file)") :
@@ -3394,10 +3402,7 @@ gdm_slave_session_start (void)
 	    g_free (errfile);
     }
 
-    gdm_debug ("gdm_slave_session_start: Session ended OK");
-
-    gdm_slave_session_stop (pid != 0 /* run_post_session */,
-			    FALSE /* no_shutdown_check */);
+    gdm_debug ("gdm_slave_session_start: Session ended OK (now all finished)");
 }
 
 
@@ -3625,7 +3630,7 @@ gdm_slave_alrm_handler (int sig)
 }
 
 /* Called on every SIGCHLD */
-static void 
+void 
 gdm_slave_child_handler (int sig)
 {
     gint status;
diff --git a/daemon/slave.h b/daemon/slave.h
index e0f5bce8..1da81ccd 100644
--- a/daemon/slave.h
+++ b/daemon/slave.h
@@ -38,6 +38,8 @@ void     gdm_slave_whack_temp_auth_file (void);
 
 gboolean gdm_slave_check_user_wants_to_log_in (const char *user);
 
+/* This is the slave child handler so that we can chain to it from elsewhere */
+void	 gdm_slave_child_handler (int sig);
 
 #endif /* GDM_SLAVE_H */
 
diff --git a/daemon/verify-pam.c b/daemon/verify-pam.c
index 5a48bd94..4e60550a 100644
--- a/daemon/verify-pam.c
+++ b/daemon/verify-pam.c
@@ -963,6 +963,8 @@ gdm_verify_cleanup (GdmDisplay *d)
 		gdm_sigchld_block_pop ();
 		gdm_sigterm_block_pop ();
 
+		pamerr = PAM_SUCCESS;
+
 		/* Close the users session */
 		if (old_opened_session) {
 			gdm_debug ("Running pam_close_session");
diff --git a/gui/greeter/gdmthemetester b/gui/greeter/gdmthemetester
index 5b06f7dd..3f420649 100755
--- a/gui/greeter/gdmthemetester
+++ b/gui/greeter/gdmthemetester
@@ -68,9 +68,9 @@ xdmcp)
 esac
 
 if [ "x$XNESTSIZE" = x ] ; then
-  eval `gdmXnest -b`
+  eval `gdmXnest -b -o "-terminate"`
 else
-  eval `gdmXnest -b -o "-geometry $XNESTSIZE"`
+  eval `gdmXnest -b -o "-geometry $XNESTSIZE -terminate""`
 fi
 eval `gdmXnest -b`
 export DISPLAY