Ensure XDMP X sever shuts down when session closes

Previously we were attempting to reconnect a greeter, however we can't trust
the X server after the session is run, so we should instead close the connection.
The XDMCP client is expected to reconnect again.

https://bugs.launchpad.net/bugs/1739787

9 files changed, 138 insertions, 14 deletions

diff --git a/src/seat-xdmcp-session.c b/src/seat-xdmcp-session.c
index ba24dc69..ebcdb706 100644
--- a/src/seat-xdmcp-session.c
+++ b/src/seat-xdmcp-session.c
@@ -18,6 +18,9 @@ struct SeatXDMCPSessionPrivate
 {
     /* Session being serviced */
     XDMCPSession *session;
+
+    /* X server using XDMCP connection */
+    XServerRemote *x_server;
 };
 
 G_DEFINE_TYPE (SeatXDMCPSession, seat_xdmcp_session, SEAT_TYPE)
@@ -37,18 +40,21 @@ static DisplayServer *
 seat_xdmcp_session_create_display_server (Seat *seat, Session *session)
 {
     XAuthority *authority;
-    gchar *host;
-    XServerRemote *x_server;
+    g_autofree gchar *host = NULL;
 
     if (strcmp (session_get_session_type (session), "x") != 0)
         return NULL;
 
+    /* Only create one server for the lifetime of this seat (XDMCP clients reconnect on logout) */
+    if (SEAT_XDMCP_SESSION (seat)->priv->x_server)
+        return NULL;
+
     authority = xdmcp_session_get_authority (SEAT_XDMCP_SESSION (seat)->priv->session);
     host = g_inet_address_to_string (xdmcp_session_get_address (SEAT_XDMCP_SESSION (seat)->priv->session));
-    x_server = x_server_remote_new (host, xdmcp_session_get_display_number (SEAT_XDMCP_SESSION (seat)->priv->session), authority);
-    g_free (host);
 
-    return DISPLAY_SERVER (x_server);
+    SEAT_XDMCP_SESSION (seat)->priv->x_server = x_server_remote_new (host, xdmcp_session_get_display_number (SEAT_XDMCP_SESSION (seat)->priv->session), authority);
+
+    return g_object_ref (DISPLAY_SERVER (SEAT_XDMCP_SESSION (seat)->priv->x_server));
 }
 
 static void
@@ -63,6 +69,7 @@ seat_xdmcp_session_finalize (GObject *object)
     SeatXDMCPSession *self = SEAT_XDMCP_SESSION (object);
 
     g_clear_object (&self->priv->session);
+    g_clear_object (&self->priv->x_server);
 
     G_OBJECT_CLASS (seat_xdmcp_session_parent_class)->finalize (object);
 }
diff --git a/src/seat.c b/src/seat.c
index a013f07f..d320a68c 100644
--- a/src/seat.c
+++ b/src/seat.c
@@ -864,6 +864,9 @@ session_stopped_cb (Session *session, Seat *seat)
         }
     }
 
+    g_signal_emit (seat, signals[SESSION_REMOVED], 0, session);
+    g_object_unref (session);
+
     /* Stop the display server if no-longer required */
     if (display_server && !display_server_get_is_stopping (display_server) &&
         !SEAT_GET_CLASS (seat)->display_server_is_used (seat, display_server))
@@ -871,9 +874,6 @@ session_stopped_cb (Session *session, Seat *seat)
         l_debug (seat, "Stopping display server, no sessions require it");
         display_server_stop (display_server);
     }
-
-    g_signal_emit (seat, signals[SESSION_REMOVED], 0, session);
-    g_object_unref (session);
 }
 
 static void
diff --git a/tests/Makefile.am b/tests/Makefile.am
index fd69cc95..9ef6b1da 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -199,6 +199,7 @@ TESTS = \
 	test-xdmcp-client-xorg-1.16 \
 	test-xdmcp-server-autologin \
 	test-xdmcp-server-login \
+	test-xdmcp-server-login-logout \
 	test-xdmcp-server-double-login \
 	test-xdmcp-server-guest \
 	test-xdmcp-server-keep-alive \
@@ -672,6 +673,7 @@ EXTRA_DIST = \
 	scripts/xdmcp-server-invalid-authentication.conf \
 	scripts/xdmcp-server-keep-alive.conf \
 	scripts/xdmcp-server-login.conf \
+	scripts/xdmcp-server-login-logout.conf \
 	scripts/xdmcp-server-open-file-descriptors.conf \
 	scripts/xdmcp-server-request-invalid-authentication.conf \
 	scripts/xdmcp-server-request-invalid-authorization.conf \
diff --git a/tests/scripts/xdmcp-server-login-logout.conf b/tests/scripts/xdmcp-server-login-logout.conf
new file mode 100644
index 00000000..3b110912
--- /dev/null
+++ b/tests/scripts/xdmcp-server-login-logout.conf
@@ -0,0 +1,64 @@
+#
+# Check that a remote X server can login via XDMCP
+#
+
+[LightDM]
+start-default-seat=false
+
+[XDMCPServer]
+enabled=true
+
+[Seat:*]
+user-session=default
+
+#?*START-DAEMON
+#?RUNNER DAEMON-START
+#?*WAIT
+
+# Start a remote X server to log in with XDMCP
+#?*START-XSERVER ARGS=":98 -query 127.0.0.1 -nolisten unix -terminate"
+#?XSERVER-98 START LISTEN-TCP NO-LISTEN-UNIX
+
+# Request to connect - daemon says OK
+#?*XSERVER-98 SEND-QUERY
+#?XSERVER-98 GOT-WILLING AUTHENTICATION-NAME="" HOSTNAME="lightdm-test" STATUS=""
+
+# Connect - daemon says OK
+#?*XSERVER-98 SEND-REQUEST ADDRESSES="127.0.0.1" AUTHORIZATION-NAMES="MIT-MAGIC-COOKIE-1"
+#?XSERVER-98 GOT-ACCEPT SESSION-ID=[0-9]+ AUTHENTICATION-NAME="" AUTHENTICATION-DATA= AUTHORIZATION-NAME="MIT-MAGIC-COOKIE-1" AUTHORIZATION-DATA=[0-9A-F]{32}
+#?*XSERVER-98 SEND-MANAGE
+
+# LightDM connects to X server
+#?XSERVER-98 ACCEPT-CONNECT
+
+# Greeter starts and connects to remote X server
+#?GREETER-X-127.0.0.1:98 START XDG_SESSION_CLASS=greeter
+#?LOGIN1 ACTIVATE-SESSION SESSION=c0
+#?XSERVER-98 ACCEPT-CONNECT
+#?GREETER-X-127.0.0.1:98 CONNECT-XSERVER
+#?GREETER-X-127.0.0.1:98 CONNECT-TO-DAEMON
+#?GREETER-X-127.0.0.1:98 CONNECTED-TO-DAEMON
+
+# Log in
+#?*GREETER-X-127.0.0.1:98 AUTHENTICATE USERNAME=have-password1
+#?GREETER-X-127.0.0.1:98 SHOW-PROMPT TEXT="Password:"
+#?*GREETER-X-127.0.0.1:98 RESPOND TEXT="password"
+#?GREETER-X-127.0.0.1:98 AUTHENTICATION-COMPLETE USERNAME=have-password1 AUTHENTICATED=TRUE
+#?*GREETER-X-127.0.0.1:98 START-SESSION
+#?GREETER-X-127.0.0.1:98 TERMINATE SIGNAL=15
+
+# Session starts
+#?SESSION-X-127.0.0.1:98 START XDG_SESSION_TYPE=x11 XDG_SESSION_DESKTOP=default USER=have-password1
+#?LOGIN1 ACTIVATE-SESSION SESSION=c1
+#?XSERVER-98 ACCEPT-CONNECT
+#?SESSION-X-127.0.0.1:98 CONNECT-XSERVER
+
+# Logout session
+#?*SESSION-X-127.0.0.1:98 LOGOUT
+
+# X server stops
+#?XSERVER-98 TERMINATE
+
+# Clean up
+#?*STOP-DAEMON
+#?RUNNER DAEMON-EXIT STATUS=0
diff --git a/tests/src/X.c b/tests/src/X.c
index 8d67a6d1..74bcbedf 100644
--- a/tests/src/X.c
+++ b/tests/src/X.c
@@ -62,6 +62,9 @@ static guint32 xdmcp_session_id = 0;
 static guint16 xdmcp_cookie_length = 0;
 static guint8 *xdmcp_cookie = NULL;
 
+/* Terminate on server reset */
+static gboolean terminate_on_reset = FALSE;
+
 static void
 cleanup (void)
 {
@@ -184,6 +187,16 @@ client_disconnected_cb (XServer *server, XClient *client)
     g_signal_handlers_disconnect_matched (client, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, NULL);
 }
 
+static void
+reset_cb (XServer *server)
+{
+    if (terminate_on_reset)
+    {
+        status_notify ("%s TERMINATE", id);
+        quit (EXIT_SUCCESS);
+    }
+}
+
 static guint8
 get_nibble (char c)
 {
@@ -458,6 +471,10 @@ main (int argc, char **argv)
             seat = argv[i+1];
             i++;
         }
+        else if (strcmp (arg, "-terminate") == 0)
+        {
+            terminate_on_reset = TRUE;
+        }
         else if (strcmp (arg, "-mir") == 0)
         {
             mir_id = argv[i+1];
@@ -504,6 +521,7 @@ main (int argc, char **argv)
     xserver = x_server_new (display_number);
     g_signal_connect (xserver, X_SERVER_SIGNAL_CLIENT_CONNECTED, G_CALLBACK (client_connected_cb), NULL);
     g_signal_connect (xserver, X_SERVER_SIGNAL_CLIENT_DISCONNECTED, G_CALLBACK (client_disconnected_cb), NULL);
+    g_signal_connect (xserver, X_SERVER_SIGNAL_RESET, G_CALLBACK (reset_cb), NULL);
 
     status_text = g_string_new ("");
     g_string_printf (status_text, "%s START", id);
diff --git a/tests/src/libsystem.c b/tests/src/libsystem.c
index 3fb8897c..62fe6480 100644
--- a/tests/src/libsystem.c
+++ b/tests/src/libsystem.c
@@ -1867,7 +1867,10 @@ xcb_disconnect (xcb_connection_t *c)
 {
     free (c->display);
     if (c->socket)
+    {
+        g_socket_close (c->socket, NULL);
         g_object_unref (c->socket);
+    }
     free (c);
 }
 
diff --git a/tests/src/x-server.c b/tests/src/x-server.c
index 3d141b36..2c9446a7 100644
--- a/tests/src/x-server.c
+++ b/tests/src/x-server.c
@@ -20,6 +20,7 @@ G_DEFINE_TYPE (XClient, x_client, G_TYPE_OBJECT)
 enum {
     X_SERVER_CLIENT_CONNECTED,
     X_SERVER_CLIENT_DISCONNECTED,
+    X_SERVER_RESET,
     X_SERVER_LAST_SIGNAL
 };
 static guint x_server_signals[X_SERVER_LAST_SIGNAL] = { 0 };
@@ -101,12 +102,29 @@ x_server_new (gint display_number)
     return server;
 }
 
-static void
-x_client_disconnected_cb (XClient *client, XServer *server)
+static gboolean
+client_read_cb (GIOChannel *channel, GIOCondition condition, gpointer data)
 {
-    g_signal_handlers_disconnect_matched (client, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, server);
-    g_hash_table_remove (server->priv->clients, client->priv->channel);
-    g_signal_emit (server, x_server_signals[X_SERVER_CLIENT_DISCONNECTED], 0, client);
+    XClient *client = data;
+    g_autofree gchar *d = NULL;
+    gsize d_length;
+
+    if (g_io_channel_read_to_end (channel, &d, &d_length, NULL) == G_IO_STATUS_NORMAL && d_length == 0)
+    {
+        XServer *server = client->priv->server;
+
+        g_signal_emit (client, x_client_signals[X_CLIENT_DISCONNECTED], 0);
+        g_signal_emit (server, x_server_signals[X_SERVER_CLIENT_DISCONNECTED], 0, client);
+
+        g_hash_table_remove (server->priv->clients, client->priv->channel);
+
+        if (g_hash_table_size (server->priv->clients) == 0)
+            g_signal_emit (server, x_server_signals[X_SERVER_RESET], 0);
+
+        return G_SOURCE_REMOVE;
+    }
+
+    return G_SOURCE_CONTINUE;
 }
 
 static gboolean
@@ -125,9 +143,9 @@ socket_connect_cb (GIOChannel *channel, GIOCondition condition, gpointer data)
 
     client = g_object_new (x_client_get_type (), NULL);
     client->priv->server = server;
-    g_signal_connect (client, X_CLIENT_SIGNAL_DISCONNECTED, G_CALLBACK (x_client_disconnected_cb), server);
     client->priv->socket = data_socket;
     client->priv->channel = g_io_channel_unix_new (g_socket_get_fd (data_socket));
+    g_io_add_watch (client->priv->channel, G_IO_IN | G_IO_HUP, client_read_cb, client);
     g_hash_table_insert (server->priv->clients, client->priv->channel, client);
 
     g_signal_emit (server, x_server_signals[X_SERVER_CLIENT_CONNECTED], 0, client);
@@ -202,4 +220,12 @@ x_server_class_init (XServerClass *klass)
                       NULL, NULL,
                       NULL,
                       G_TYPE_NONE, 1, x_client_get_type ());
+    x_server_signals[X_SERVER_RESET] =
+        g_signal_new (X_SERVER_SIGNAL_RESET,
+                      G_TYPE_FROM_CLASS (klass),
+                      G_SIGNAL_RUN_LAST,
+                      G_STRUCT_OFFSET (XServerClass, reset),
+                      NULL, NULL,
+                      NULL,
+                      G_TYPE_NONE, 0);
 }
diff --git a/tests/src/x-server.h b/tests/src/x-server.h
index 7ce8c3fe..32f00da9 100644
--- a/tests/src/x-server.h
+++ b/tests/src/x-server.h
@@ -10,6 +10,7 @@ G_BEGIN_DECLS
 
 #define X_SERVER_SIGNAL_CLIENT_CONNECTED    "client-connected"
 #define X_SERVER_SIGNAL_CLIENT_DISCONNECTED "client-disconnected"
+#define X_SERVER_SIGNAL_RESET               "reset"
 
 typedef struct XClientPrivate XClientPrivate;
 
@@ -38,6 +39,7 @@ typedef struct
    GObjectClass parent_class;
    void (*client_connected)(XServer *server, XClient *client);
    void (*client_disconnected)(XServer *server, XClient *client);
+   void (*reset)(XServer *server);
 } XServerClass;
 
 GType x_server_get_type (void);
diff --git a/tests/test-xdmcp-server-login-logout b/tests/test-xdmcp-server-login-logout
new file mode 100755
index 00000000..66425bfb
--- /dev/null
+++ b/tests/test-xdmcp-server-login-logout
@@ -0,0 +1,2 @@
+#!/bin/sh
+./src/dbus-env ./src/test-runner xdmcp-server-login-logout test-gobject-greeter