[gdm/gnome-3-38] session-worker: Don't switch back VTs until session is fully exited

[Ray Strode <halfline src gnome org>]

commit 9b6d9b24a5f69674447c7bc9aacfab0988b914bd
Author: Ray Strode <rstrode redhat com>
Date:   Thu Dec 10 15:14:20 2020 -0500

    session-worker: Don't switch back VTs until session is fully exited
    
    There's a race condition on shutdown where the session worker is
    switching VTs back to the initial VT at the same time as the session
    exit is being processed.
    
    This means that manager may try to start a login screen (because of
    the VT switch) when autologin is enabled when there shouldn't be a
    login screen.
    
    This commit makes sure both the PostSession script, and session-exited
    signal emission are complete before initiating the VT switch back
    to the initial VT.
    
    https://gitlab.gnome.org/GNOME/gdm/-/issues/660

 daemon/gdm-session-worker.c     | 93 ++++++++++++++++++++++++++++++++++-------
 daemon/meson.build              |  7 ++++
 daemon/org.freedesktop.DBus.xml | 12 ++++++
 3 files changed, 96 insertions(+), 16 deletions(-)
---
diff --git a/daemon/gdm-session-worker.c b/daemon/gdm-session-worker.c
index d2cfde515..36c3e5dfd 100644
--- a/daemon/gdm-session-worker.c
+++ b/daemon/gdm-session-worker.c
@@ -66,6 +66,7 @@
 #include "gdm-pam-extensions.h"
 #endif
 
+#include "gdm-dbus-glue.h"
 #include "gdm-session-worker.h"
 #include "gdm-session-glue.h"
 #include "gdm-session.h"
@@ -1051,18 +1052,6 @@ gdm_session_worker_uninitialize_pam (GdmSessionWorker *worker,
 
         gdm_session_worker_stop_auditor (worker);
 
-        /* If user-display-server is not enabled the login_vt is always
-         * identical to the session_vt. So in that case we never need to
-         * do a VT switch. */
-#ifdef ENABLE_USER_DISPLAY_SERVER
-        if (g_strcmp0 (worker->priv->display_seat_id, "seat0") == 0) {
-                /* Switch to the login VT if we are not the login screen. */
-                if (worker->priv->session_vt != GDM_INITIAL_VT) {
-                        jump_to_vt (worker, GDM_INITIAL_VT);
-                }
-        }
-#endif
-
         worker->priv->session_vt = 0;
 
         g_debug ("GdmSessionWorker: state NONE");
@@ -1775,6 +1764,53 @@ run_script (GdmSessionWorker *worker,
                                worker->priv->x11_authority_file);
 }
 
+static void
+wait_until_dbus_signal_emission_to_manager_finishes (GdmSessionWorker *worker)
+{
+        g_autoptr (GdmDBusPeer) peer_proxy = NULL;
+        g_autoptr (GError) error = NULL;
+        gboolean pinged;
+
+        peer_proxy = gdm_dbus_peer_proxy_new_sync (worker->priv->connection,
+                                                   G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES,
+                                                   NULL,
+                                                   "/org/freedesktop/DBus",
+                                                   NULL,
+                                                   &error);
+
+        if (peer_proxy == NULL) {
+                g_debug ("GdmSessionWorker: could not create peer proxy to daemon: %s",
+                         error->message);
+                return;
+        }
+
+        pinged = gdm_dbus_peer_call_ping_sync (peer_proxy, NULL, &error);
+
+        if (!pinged) {
+                g_debug ("GdmSessionWorker: could not ping daemon: %s",
+                         error->message);
+                return;
+        }
+}
+
+static void
+jump_back_to_initial_vt (GdmSessionWorker *worker)
+{
+        if (worker->priv->session_vt == 0)
+                return;
+
+        if (worker->priv->session_vt == GDM_INITIAL_VT)
+                return;
+
+        if (g_strcmp0 (worker->priv->display_seat_id, "seat0") != 0)
+                return;
+
+#ifdef ENABLE_USER_DISPLAY_SERVER
+        jump_to_vt (worker, GDM_INITIAL_VT);
+        worker->priv->session_vt = 0;
+#endif
+}
+
 static void
 session_worker_child_watch (GPid              pid,
                             int               status,
@@ -1789,18 +1825,40 @@ session_worker_child_watch (GPid              pid,
                  : WIFSIGNALED (status) ? WTERMSIG (status)
                  : -1);
 
-
         gdm_session_worker_uninitialize_pam (worker, PAM_SUCCESS);
 
+        worker->priv->child_pid = -1;
+        worker->priv->child_watch_id = 0;
+        run_script (worker, GDMCONFDIR "/PostSession");
+
         gdm_dbus_worker_emit_session_exited (GDM_DBUS_WORKER (worker),
                                              worker->priv->service,
                                              status);
 
         killpg (pid, SIGHUP);
 
-        worker->priv->child_pid = -1;
-        worker->priv->child_watch_id = 0;
-        run_script (worker, GDMCONFDIR "/PostSession");
+        /* FIXME: It's important to give the manager an opportunity to process the
+         * session-exited emission above before switching VTs.
+         *
+         * This is because switching VTs makes the manager try to put a login screen
+         * up on VT 1, but it may actually want to try to auto login again in response
+         * to session-exited.
+         *
+         * This function just does a manager roundtrip over the bus to make sure the
+         * signal has been dispatched before jumping.
+         *
+         * Ultimately, we may want to improve the manager<->worker interface.
+         *
+         * See:
+         *
+         * https://gitlab.gnome.org/GNOME/gdm/-/merge_requests/123
+         *
+         * for some ideas and more discussion.
+         *
+         */
+        wait_until_dbus_signal_emission_to_manager_finishes (worker);
+
+        jump_back_to_initial_vt (worker);
 }
 
 static void
@@ -2424,6 +2482,7 @@ gdm_session_worker_open_session (GdmSessionWorker  *worker,
  out:
         if (error_code != PAM_SUCCESS) {
                 gdm_session_worker_uninitialize_pam (worker, error_code);
+                worker->priv->session_vt = 0;
                 return FALSE;
         }
 
@@ -3549,6 +3608,8 @@ gdm_session_worker_finalize (GObject *object)
                 gdm_session_worker_uninitialize_pam (worker, PAM_SUCCESS);
         }
 
+        jump_back_to_initial_vt (worker);
+
         g_object_unref (worker->priv->user_settings);
         g_free (worker->priv->service);
         g_free (worker->priv->x11_display_name);
diff --git a/daemon/meson.build b/daemon/meson.build
index 72c7c3c12..2e61b6447 100644
--- a/daemon/meson.build
+++ b/daemon/meson.build
@@ -1,4 +1,10 @@
 # D-Bus interfaces
+dbus_gen = gnome.gdbus_codegen('gdm-dbus-glue',
+  'org.freedesktop.DBus.xml',
+  namespace: 'GdmDBus',
+  interface_prefix: 'org.freedesktop.DBus',
+  autocleanup: 'all',
+)
 display_dbus_gen = gnome.gdbus_codegen('gdm-display-glue',
   'gdm-display.xml',
   namespace: 'GdmDBus',
@@ -87,6 +93,7 @@ gdm_session_worker_src = [
   'gdm-session-worker-job.c',
   'gdm-session-worker-common.c',
   'gdm-dbus-util.c',
+  dbus_gen,
   session_dbus_gen,
   session_worker_dbus_gen,
   gdm_session_enums,
diff --git a/daemon/org.freedesktop.DBus.xml b/daemon/org.freedesktop.DBus.xml
new file mode 100644
index 000000000..5e0814bde
--- /dev/null
+++ b/daemon/org.freedesktop.DBus.xml
@@ -0,0 +1,12 @@
+<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN"
+"http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd";>
+<node>
+  <interface name="org.freedesktop.DBus.Peer">
+    <method name="GetMachineId">
+      <arg direction="out" type="s"/>
+    </method>
+    <method name="Ping">
+    </method>
+  </interface>
+</node>
+