From 0ff911467265831006aac6216060dbecff84c1cb Mon Sep 17 00:00:00 2001

From: Hans de Goede <hdegoede@redhat.com>

Date: Tue, 4 Sep 2018 10:56:45 +0200

Subject: [PATCH 30/51] daemon: Move the waiting the session to have taken over

 the fb to gdm-local-display-factory

Commit 708618746683 ("gdm-wayland-session,gdm-x-session: register after

delay") delayed displays changing their status from PREPARED to MANAGED

so that their status would not change until the session has had a change

to install its own framebuffer and tell the GPU to scanout this new fb.

Commit 74ee77717df7 ("local-display-factory: defer killing greeter until

new session registers") uses this to avoid a flicker when transitioning

from the greeter to the user-session by deferring the stopping of the

greeter-session until the new display moves to the MANAGED state.

But this only works when transitioning to a new user-session, when moving

to an existing user-session (fast user switching) the display already

is in MANAGED state and instead of deferring the stopping of the greeter

commit 74ee77717df7 causes us to now never stop the greeter-session.

This commit fixes this by starting a timeout when switching away from

the initial-vt and letting that timeout stop the greeter-session.

This commit removes the finish_waiting_displays_on_seat() call when the

display's status changes to MANAGED, so that we still only have one code

path stopping the greeter and not two.

This means we also no longer need to delay registering the display. So this

commit removes the code adding the delay (reverts commit 74ee77717df7).

Note this commit uses a delay of 10 seconds, rather then 2 seconds. The

transition to a new user-session takes about 8 seconds on my budget

Apollo Lake based laptop (with SSD).

Note this all really is a workaround, the proper solution for this would

be able to tell the kernel to keep the greeter framebuffer around until

a new framebuffer is installed. There is a patch to add a new unref_fb

ioctl for this: https://www.spinics.net/lists/dri-devel/msg140912.html .

We need to get this patch upstream and teach mutter to use it.

---

 daemon/gdm-local-display-factory.c | 29 ++++++++++++++++++++++++++---

 daemon/gdm-wayland-session.c       | 23 +++++++----------------

 daemon/gdm-x-session.c             | 25 ++++++++-----------------

 3 files changed, 41 insertions(+), 36 deletions(-)

diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c

index bc6ac6855..be6b377be 100644

--- a/daemon/gdm-local-display-factory.c

+++ b/daemon/gdm-local-display-factory.c

@@ -22,76 +22,78 @@

 #include <stdlib.h>

 #include <stdio.h>

 #include <glib.h>

 #include <glib/gi18n.h>

 #include <glib-object.h>

 #include <gio/gio.h>

 #include <systemd/sd-login.h>

 #include "gdm-common.h"

 #include "gdm-manager.h"

 #include "gdm-display-factory.h"

 #include "gdm-local-display-factory.h"

 #include "gdm-local-display-factory-glue.h"

 #include "gdm-settings-keys.h"

 #include "gdm-settings-direct.h"

 #include "gdm-display-store.h"

 #include "gdm-local-display.h"

 #include "gdm-legacy-display.h"

 #define GDM_LOCAL_DISPLAY_FACTORY_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), GDM_TYPE_LOCAL_DISPLAY_FACTORY, GdmLocalDisplayFactoryPrivate))

 #define GDM_DBUS_PATH                       "/org/gnome/DisplayManager"

 #define GDM_LOCAL_DISPLAY_FACTORY_DBUS_PATH GDM_DBUS_PATH "/LocalDisplayFactory"

 #define GDM_MANAGER_DBUS_NAME               "org.gnome.DisplayManager.LocalDisplayFactory"

 #define MAX_DISPLAY_FAILURES 5

+#define WAIT_TO_FINISH_TIMEOUT 10 /* seconds */

 struct GdmLocalDisplayFactoryPrivate

 {

         GdmDBusLocalDisplayFactory *skeleton;

         GDBusConnection *connection;

         GHashTable      *used_display_numbers;

         /* FIXME: this needs to be per seat? */

         guint            num_failures;

         guint            seat_new_id;

         guint            seat_removed_id;

 #if defined(ENABLE_WAYLAND_SUPPORT) && defined(ENABLE_USER_DISPLAY_SERVER)

         char            *tty_of_active_vt;

         guint            active_vt_watch_id;

+        guint            wait_to_finish_timeout_id;

 #endif

 };

 enum {

         PROP_0,

 };

 static void     gdm_local_display_factory_class_init    (GdmLocalDisplayFactoryClass *klass);

 static void     gdm_local_display_factory_init          (GdmLocalDisplayFactory      *factory);

 static void     gdm_local_display_factory_finalize      (GObject                     *object);

 static GdmDisplay *create_display                       (GdmLocalDisplayFactory      *factory,

                                                          const char                  *seat_id,

                                                          const char                  *session_type,

                                                          gboolean                    initial_display);

 static void     on_display_status_changed               (GdmDisplay                  *display,

                                                          GParamSpec                  *arg1,

                                                          GdmLocalDisplayFactory      *factory);

 static gboolean gdm_local_display_factory_sync_seats    (GdmLocalDisplayFactory *factory);

 static gpointer local_display_factory_object = NULL;

 static gboolean lookup_by_session_id (const char *id,

                                       GdmDisplay *display,

                                       gpointer    user_data);

 G_DEFINE_TYPE (GdmLocalDisplayFactory, gdm_local_display_factory, GDM_TYPE_DISPLAY_FACTORY)

 GQuark

 gdm_local_display_factory_error_quark (void)

@@ -354,61 +356,60 @@ on_display_status_changed (GdmDisplay             *display,

                 /* Create a new equivalent display if it was static */

                 if (is_local) {

                         factory->priv->num_failures++;

                         if (factory->priv->num_failures > MAX_DISPLAY_FAILURES) {

                                 /* oh shit */

                                 g_warning ("GdmLocalDisplayFactory: maximum number of X display failures reached: check X server log for errors");

                         } else {

 #ifdef ENABLE_WAYLAND_SUPPORT

                                 if (g_strcmp0 (session_type, "wayland") == 0) {

                                         g_free (session_type);

                                         session_type = NULL;

                                         /* workaround logind race for now

                                          * bug 1643874

                                          */

                                         g_usleep (2 * G_USEC_PER_SEC);

                                 }

 #endif

                                 create_display (factory, seat_id, session_type, is_initial);

                         }

                 }

                 break;

         case GDM_DISPLAY_UNMANAGED:

                 break;

         case GDM_DISPLAY_PREPARED:

                 break;

         case GDM_DISPLAY_MANAGED:

-                finish_waiting_displays_on_seat (factory, seat_id);

                 break;

         case GDM_DISPLAY_WAITING_TO_FINISH:

                 break;

         default:

                 g_assert_not_reached ();

                 break;

         }

         g_free (seat_id);

         g_free (session_type);

         g_free (session_class);

 }

 static gboolean

 lookup_by_seat_id (const char *id,

                    GdmDisplay *display,

                    gpointer    user_data)

 {

         const char *looking_for = user_data;

         char *current;

         gboolean res;

         g_object_get (G_OBJECT (display), "seat-id", &current, NULL);

         res = g_strcmp0 (current, looking_for) == 0;

         g_free(current);

         return res;

 }

@@ -593,83 +594,101 @@ on_seat_new (GDBusConnection *connection,

 }

 static void

 on_seat_removed (GDBusConnection *connection,

                  const gchar     *sender_name,

                  const gchar     *object_path,

                  const gchar     *interface_name,

                  const gchar     *signal_name,

                  GVariant        *parameters,

                  gpointer         user_data)

 {

         const char *seat;

         g_variant_get (parameters, "(&s&o)", &seat, NULL);

         delete_display (GDM_LOCAL_DISPLAY_FACTORY (user_data), seat);

 }

 #if defined(ENABLE_WAYLAND_SUPPORT) && defined(ENABLE_USER_DISPLAY_SERVER)

 static gboolean

 lookup_by_session_id (const char *id,

                       GdmDisplay *display,

                       gpointer    user_data)

 {

         const char *looking_for = user_data;

         const char *current;

         current = gdm_display_get_session_id (display);

         return g_strcmp0 (current, looking_for) == 0;

 }

+static gboolean

+wait_to_finish_timeout (GdmLocalDisplayFactory *factory)

+{

+        finish_waiting_displays_on_seat (factory, "seat0");

+        factory->priv->wait_to_finish_timeout_id = 0;

+        return G_SOURCE_REMOVE;

+}

+

 static void

-maybe_stop_greeter_in_background (GdmDisplay *display)

+maybe_stop_greeter_in_background (GdmLocalDisplayFactory *factory,

+                                  GdmDisplay             *display)

 {

         g_autofree char *display_session_type = NULL;

         if (gdm_display_get_status (display) != GDM_DISPLAY_MANAGED) {

                 g_debug ("GdmLocalDisplayFactory: login window not in managed state, so ignoring");

                 return;

         }

         g_object_get (G_OBJECT (display),

                       "session-type", &display_session_type,

                       NULL);

         /* we can only stop greeter for wayland sessions, since

          * X server would jump back on exit */

         if (g_strcmp0 (display_session_type, "wayland") != 0) {

                 g_debug ("GdmLocalDisplayFactory: login window is running on Xorg, so ignoring");

                 return;

         }

         g_debug ("GdmLocalDisplayFactory: killing login window once its unused");

         g_object_set (G_OBJECT (display), "status", GDM_DISPLAY_WAITING_TO_FINISH, NULL);

+

+        /* We stop the greeter after a timeout to avoid flicker */

+        if (factory->priv->wait_to_finish_timeout_id != 0)

+                g_source_remove (factory->priv->wait_to_finish_timeout_id);

+

+        factory->priv->wait_to_finish_timeout_id =

+                g_timeout_add_seconds (WAIT_TO_FINISH_TIMEOUT,

+                                       (GSourceFunc)wait_to_finish_timeout,

+                                       factory);

 }

 static gboolean

 on_vt_changed (GIOChannel    *source,

                GIOCondition   condition,

                GdmLocalDisplayFactory *factory)

 {

         GIOStatus status;

         static const char *tty_of_initial_vt = "tty" GDM_INITIAL_VT;

         g_autofree char *tty_of_previous_vt = NULL;

         g_autofree char *tty_of_active_vt = NULL;

         g_autofree char *login_session_id = NULL;

         g_autofree char *active_session_id = NULL;

         const char *session_type = NULL;

         int ret;

         g_debug ("GdmLocalDisplayFactory: received VT change event");

         g_io_channel_seek_position (source, 0, G_SEEK_SET, NULL);

         if (condition & G_IO_PRI) {

                 g_autoptr (GError) error = NULL;

                 status = g_io_channel_read_line (source, &tty_of_active_vt, NULL, NULL, &error);

                 if (error != NULL) {

                         g_warning ("could not read active VT from kernel: %s", error->message);

                 }

                 switch (status) {

                         case G_IO_STATUS_ERROR:

                             return G_SOURCE_REMOVE;

                         case G_IO_STATUS_EOF:

@@ -709,61 +728,61 @@ on_vt_changed (GIOChannel    *source,

                 return G_SOURCE_CONTINUE;

         }

         g_debug ("GdmLocalDisplayFactory: VT changed from %s to %s",

                  tty_of_previous_vt, factory->priv->tty_of_active_vt);

         /* if the old VT was running a wayland login screen kill it

          */

         if (gdm_get_login_window_session_id ("seat0", &login_session_id)) {

                 unsigned int vt;

                 ret = sd_session_get_vt (login_session_id, &vt;;

                 if (ret == 0 && vt != 0) {

                         g_autofree char *tty_of_login_window_vt = NULL;

                         tty_of_login_window_vt = g_strdup_printf ("tty%u", vt);

                         g_debug ("GdmLocalDisplayFactory: tty of login window is %s", tty_of_login_window_vt);

                         if (g_strcmp0 (tty_of_login_window_vt, tty_of_previous_vt) == 0) {

                                 GdmDisplayStore *store;

                                 GdmDisplay *display;

                                 g_debug ("GdmLocalDisplayFactory: VT switched from login window");

                                 store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory));

                                 display = gdm_display_store_find (store,

                                                                   lookup_by_session_id,

                                                                   (gpointer) login_session_id);

                                 if (display != NULL)

-                                        maybe_stop_greeter_in_background (display);

+                                        maybe_stop_greeter_in_background (factory, display);

                         } else {

                                 g_debug ("GdmLocalDisplayFactory: VT not switched from login window");

                         }

                 }

         }

         /* if user jumped back to initial vt and it's empty put a login screen

          * on it (unless a login screen is already running elsewhere, then

          * jump to that login screen)

          */

         if (strcmp (factory->priv->tty_of_active_vt, tty_of_initial_vt) != 0) {

                 g_debug ("GdmLocalDisplayFactory: active VT is not initial VT, so ignoring");

                 return G_SOURCE_CONTINUE;

         }

         ret = sd_seat_get_active ("seat0", &active_session_id, NULL);

         if (ret == 0) {

                 g_autofree char *state = NULL;

                 ret = sd_session_get_state (active_session_id, &state);

                 /* if there's something already running on the active VT then bail */

                 if (ret == 0 && g_strcmp0 (state, "closing") != 0) {

                         g_debug ("GdmLocalDisplayFactory: initial VT is in use, so ignoring");

                         return G_SOURCE_CONTINUE;

                 }

         }

         if (gdm_local_display_factory_use_wayland ())

                 session_type = "wayland";

@@ -803,60 +822,64 @@ gdm_local_display_factory_start_monitor (GdmLocalDisplayFactory *factory)

                                                                              g_object_unref);

 #if defined(ENABLE_WAYLAND_SUPPORT) && defined(ENABLE_USER_DISPLAY_SERVER)

         io_channel = g_io_channel_new_file ("/sys/class/tty/tty0/active", "r", NULL);

         if (io_channel != NULL) {

                 factory->priv->active_vt_watch_id =

                         g_io_add_watch (io_channel,

                                         G_IO_PRI,

                                         (GIOFunc)

                                         on_vt_changed,

                                         factory);

         }

 #endif

 }

 static void

 gdm_local_display_factory_stop_monitor (GdmLocalDisplayFactory *factory)

 {

         if (factory->priv->seat_new_id) {

                 g_dbus_connection_signal_unsubscribe (factory->priv->connection,

                                                       factory->priv->seat_new_id);

                 factory->priv->seat_new_id = 0;

         }

         if (factory->priv->seat_removed_id) {

                 g_dbus_connection_signal_unsubscribe (factory->priv->connection,

                                                       factory->priv->seat_removed_id);

                 factory->priv->seat_removed_id = 0;

         }

 #if defined(ENABLE_WAYLAND_SUPPORT) && defined(ENABLE_USER_DISPLAY_SERVER)

+        if (factory->priv->wait_to_finish_timeout_id != 0) {

+                g_source_remove (factory->priv->wait_to_finish_timeout_id);

+                factory->priv->wait_to_finish_timeout_id = 0;

+        }

         if (factory->priv->active_vt_watch_id) {

                 g_source_remove (factory->priv->active_vt_watch_id);

                 factory->priv->active_vt_watch_id = 0;

         }

         g_clear_pointer (&factory->priv->tty_of_active_vt, g_free);

 #endif

 }

 static void

 on_display_added (GdmDisplayStore        *display_store,

                   const char             *id,

                   GdmLocalDisplayFactory *factory)

 {

         GdmDisplay *display;

         display = gdm_display_store_lookup (display_store, id);

         if (display != NULL) {

                 g_signal_connect_object (display, "notify::status",

                                          G_CALLBACK (on_display_status_changed),

                                          factory,

                                          0);

                 g_object_weak_ref (G_OBJECT (display), (GWeakNotify)on_display_disposed, factory);

         }

 }

 static void

 on_display_removed (GdmDisplayStore        *display_store,

diff --git a/daemon/gdm-wayland-session.c b/daemon/gdm-wayland-session.c

index de1991b34..94f49e19c 100644

--- a/daemon/gdm-wayland-session.c

+++ b/daemon/gdm-wayland-session.c

@@ -427,75 +427,60 @@ init_state (State **state)

         static State state_allocation;

         *state = &state_allocation;

 }

 static void

 clear_state (State **out_state)

 {

         State *state = *out_state;

         g_clear_object (&state->cancellable);

         g_clear_object (&state->bus_connection);

         g_clear_object (&state->session_subprocess);

         g_clear_pointer (&state->environment, g_strfreev);

         g_clear_pointer (&state->main_loop, g_main_loop_unref);

         *out_state = NULL;

 }

 static gboolean

 on_sigterm (State *state)

 {

         g_cancellable_cancel (state->cancellable);

         if (g_main_loop_is_running (state->main_loop)) {

                 g_main_loop_quit (state->main_loop);

         }

         return G_SOURCE_CONTINUE;

 }

-static gboolean

-on_registration_delay_complete (State *state)

-{

-        gboolean ret;

-

-        ret = register_display (state, state->cancellable);

-

-        if (!ret) {

-                g_printerr ("Unable to register display with display manager\n");

-                g_main_loop_quit (state->main_loop);

-        }

-

-        return G_SOURCE_REMOVE;

-}

-

 int

 main (int    argc,

       char **argv)

 {

         State           *state = NULL;

         GOptionContext  *context = NULL;

         static char    **args = NULL;

         gboolean         debug = FALSE;

         gboolean         ret;

         int              exit_status = EX_OK;

         static GOptionEntry entries []   = {

                 { G_OPTION_REMAINING, 0, 0, G_OPTION_ARG_STRING_ARRAY, &args, "", "" },

                 { NULL }

         };

         bindtextdomain (GETTEXT_PACKAGE, GNOMELOCALEDIR);

         textdomain (GETTEXT_PACKAGE);

         setlocale (LC_ALL, "");

         gdm_log_init ();

         context = g_option_context_new (_("GNOME Display Manager Wayland Session Launcher"));

         g_option_context_add_main_entries (context, entries, NULL);

         g_option_context_parse (context, &argc, &argv, NULL);

         g_option_context_free (context);

         if (args == NULL || args[0] == NULL || args[1] != NULL) {

                 g_warning ("gdm-wayland-session takes one argument (the session)");

                 exit_status = EX_USAGE;

@@ -516,49 +501,55 @@ main (int    argc,

         }

         gdm_settings_direct_get_boolean (GDM_KEY_DEBUG, &debug);

         state->debug_enabled = debug;

         gdm_log_set_debug (debug);

         state->main_loop = g_main_loop_new (NULL, FALSE);

         state->cancellable = g_cancellable_new ();

         g_unix_signal_add (SIGTERM, (GSourceFunc) on_sigterm, state);

         ret = spawn_bus (state, state->cancellable);

         if (!ret) {

                 g_printerr ("Unable to run session message bus\n");

                 exit_status = EX_SOFTWARE;

                 goto out;

         }

         import_environment (state, state->cancellable);

         ret = spawn_session (state, state->cancellable);

         if (!ret) {

                 g_printerr ("Unable to run session\n");

                 exit_status = EX_SOFTWARE;

                 goto out;

         }

-        g_timeout_add_seconds (2, (GSourceFunc) on_registration_delay_complete, state);

+        ret = register_display (state, state->cancellable);

+

+        if (!ret) {

+                g_printerr ("Unable to register display with display manager\n");

+                exit_status = EX_SOFTWARE;

+                goto out;

+        }

         g_main_loop_run (state->main_loop);

         /* Only use exit status of session if we're here because it exit */

         if (state->session_subprocess == NULL) {

                 exit_status = state->session_exit_status;

         }

 out:

         if (state != NULL) {

                 signal_subprocesses (state);

                 wait_on_subprocesses (state);

                 clear_state (&state);

         }

         return exit_status;

 }

diff --git a/daemon/gdm-x-session.c b/daemon/gdm-x-session.c

index 412999cf5..3b2fcef47 100644

--- a/daemon/gdm-x-session.c

+++ b/daemon/gdm-x-session.c

@@ -783,75 +783,60 @@ init_state (State **state)

 }

 static void

 clear_state (State **out_state)

 {

         State *state = *out_state;

         g_clear_object (&state->cancellable);

         g_clear_object (&state->bus_connection);

         g_clear_object (&state->session_subprocess);

         g_clear_object (&state->x_subprocess);

         g_clear_pointer (&state->environment, g_strfreev);

         g_clear_pointer (&state->auth_file, g_free);

         g_clear_pointer (&state->display_name, g_free);

         g_clear_pointer (&state->main_loop, g_main_loop_unref);

         *out_state = NULL;

 }

 static gboolean

 on_sigterm (State *state)

 {

         g_cancellable_cancel (state->cancellable);

         if (g_main_loop_is_running (state->main_loop)) {

                 g_main_loop_quit (state->main_loop);

         }

         return G_SOURCE_CONTINUE;

 }

-static gboolean

-on_registration_delay_complete (State *state)

-{

-        gboolean ret;

-

-        ret = register_display (state, state->cancellable);

-

-        if (!ret) {

-                g_printerr ("Unable to register display with display manager\n");

-                g_main_loop_quit (state->main_loop);

-        }

-

-        return G_SOURCE_REMOVE;

-}

-

 int

 main (int    argc,

       char **argv)

 {

         State           *state = NULL;

         GOptionContext  *context = NULL;

         static char    **args = NULL;

         static gboolean  run_script = FALSE;

         static gboolean  allow_remote_connections = FALSE;

         gboolean         debug = FALSE;

         gboolean         ret;

         int              exit_status = EX_OK;

         static GOptionEntry entries []   = {

                 { "run-script", 'r', 0, G_OPTION_ARG_NONE, &run_script, N_("Run program through /etc/gdm/Xsession wrapper script"), NULL },

                 { "allow-remote-connections", 'a', 0, G_OPTION_ARG_NONE, &allow_remote_connections, N_("Listen on TCP socket"), NULL },

                 { G_OPTION_REMAINING, 0, 0, G_OPTION_ARG_STRING_ARRAY, &args, "", "" },

                 { NULL }

         };

         bindtextdomain (GETTEXT_PACKAGE, GNOMELOCALEDIR);

         textdomain (GETTEXT_PACKAGE);

         setlocale (LC_ALL, "");

         gdm_log_init ();

         context = g_option_context_new (_("GNOME Display Manager X Session Launcher"));

         g_option_context_add_main_entries (context, entries, NULL);

         g_option_context_parse (context, &argc, &argv, NULL);

         g_option_context_free (context);

@@ -884,57 +869,63 @@ main (int    argc,

         state->cancellable = g_cancellable_new ();

         g_unix_signal_add (SIGTERM, (GSourceFunc) on_sigterm, state);

         ret = spawn_x_server (state, allow_remote_connections, state->cancellable);

         if (!ret) {

                 g_printerr ("Unable to run X server\n");

                 exit_status = EX_SOFTWARE;

                 goto out;

         }

         ret = spawn_bus (state, state->cancellable);

         if (!ret) {

                 g_printerr ("Unable to run session message bus\n");

                 exit_status = EX_SOFTWARE;

                 goto out;

         }

         import_environment (state, state->cancellable);

         ret = update_bus_environment (state, state->cancellable);

         if (!ret) {

                 g_printerr ("Unable to update bus environment\n");

                 exit_status = EX_SOFTWARE;

                 goto out;

         }

+        ret = register_display (state, state->cancellable);

+

+        if (!ret) {

+                g_printerr ("Unable to register display with display manager\n");

+                exit_status = EX_SOFTWARE;

+                goto out;

+        }

+

         ret = spawn_session (state, run_script, state->cancellable);

         if (!ret) {

                 g_printerr ("Unable to run session\n");

                 exit_status = EX_SOFTWARE;

                 goto out;

         }

-        g_timeout_add_seconds (2, (GSourceFunc) on_registration_delay_complete, state);

-

         g_main_loop_run (state->main_loop);

         /* Only use exit status of session if we're here because it exit */

         if (state->session_subprocess == NULL) {

                 exit_status = state->session_exit_status;

         }

 out:

         if (state != NULL) {

                 signal_subprocesses (state);

                 wait_on_subprocesses (state);

                 clear_state (&state);

         }

         return exit_status;

 }

-- 

2.27.0