[gdm/benzea/wait-seat-graphical: 12/12] local-display-factory: Wait for seats to become graphical

[Benjamin Berg <bberg src gnome org>]

commit 1ca01d8eaaa4cb959ec3aa586ebf9b1f4068b6eb
Author: Benjamin Berg <bberg redhat com>
Date:   Tue Feb 9 15:43:05 2021 +0100

    local-display-factory: Wait for seats to become graphical
    
    It may happen that seats are not graphical initially because the DRM
    device is not ready yet. In that case, ignore the seat and wait for the
    CanGraphical property notification in order to add it at that point.
    
    However, there are some rare cases where CanGraphical will never turn
    TRUE. To catch these, add a timeout and fall back to simply trying to
    bring seat0 up after 10 seconds. When we do so, go directly for the X11
    fallback as wayland is unlikely to be functional.
    
    Fixes: #662

 daemon/gdm-local-display-factory.c | 110 +++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)
---
diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c
index d9347e982..41f4aec74 100644
--- a/daemon/gdm-local-display-factory.c
+++ b/daemon/gdm-local-display-factory.c
@@ -62,6 +62,10 @@ struct _GdmLocalDisplayFactory
 
         guint            seat_new_id;
         guint            seat_removed_id;
+        guint            seat_prop_changed_id;
+
+        gboolean         force_graphical;
+        guint            force_graphical_timeout_id;
 
 #if defined(ENABLE_USER_DISPLAY_SERVER)
         unsigned int     active_vt;
@@ -449,16 +453,37 @@ lookup_prepared_display_by_seat_id (const char *id,
         return lookup_by_seat_id (id, display, user_data);
 }
 
+static int
+force_graphical_timeout_cb (gpointer user_data)
+{
+        GdmLocalDisplayFactory *factory = user_data;
+
+        factory->force_graphical_timeout_id = 0;
+
+        /* Simply try to re-add seat0. If it is there already (i.e. CanGraphical
+         * turned TRUE, then we'll find it and it will not be created again.
+         */
+        factory->force_graphical = TRUE;
+        create_display (factory, "seat0");
+
+        return G_SOURCE_REMOVE;
+}
+
 static GdmDisplay *
 create_display (GdmLocalDisplayFactory *factory,
                 const char             *seat_id)
 {
         gboolean is_initial;
+        int can_graphical;
         const char *session_type = NULL;
         GdmDisplayStore *store;
         GdmDisplay      *display = NULL;
         g_autofree char *login_session_id = NULL;
 
+        can_graphical = sd_seat_can_graphical (seat_id);
+        if (can_graphical < 0)
+                return NULL;
+
         if (g_strcmp0 (seat_id, "seat0") == 0) {
                 is_initial = TRUE;
                 if (factory->num_failures == 0 && gdm_local_display_factory_use_wayland ())
@@ -467,6 +492,31 @@ create_display (GdmLocalDisplayFactory *factory,
                 is_initial = FALSE;
         }
 
+        /* For seat0, we have a fallback logic to still try start it after 10s,
+         * i.e. we continue similar to as if CanGraphical was set.
+         * This is ugly, but it means we'll come up eventually in some
+         * scenarios where no master device is present.
+         * Note that we'll force an X11 fallback even though there might be
+         * cases where an FB device is present and simply not marked as
+         * master-of-seat. In these cases, this should likely be fixed in
+         * systemd.
+         */
+        if (is_initial && !can_graphical) {
+                if (!factory->force_graphical && !factory->force_graphical_timeout_id)
+                        factory->force_graphical_timeout_id = g_timeout_add_seconds (10, 
force_graphical_timeout_cb, factory);
+
+                /* It is not yet time to force graphical mode. */
+                if (!factory->force_graphical)
+                        return NULL;
+
+                g_warning ("GdmLocalDisplayFactory: Assuming we can use seat0 for X11 even though it is not 
graphical!");
+                can_graphical = TRUE;
+                session_type = NULL;
+        }
+
+        if (!can_graphical)
+                return NULL;
+
         g_debug ("GdmLocalDisplayFactory: %s login display for seat %s requested",
                  session_type? : "X11", seat_id);
         store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory));
@@ -615,6 +665,49 @@ on_seat_removed (GDBusConnection *connection,
         delete_display (GDM_LOCAL_DISPLAY_FACTORY (user_data), seat);
 }
 
+static void
+on_seat_prop_changed (GDBusConnection *connection,
+                      const gchar     *sender_name,
+                      const gchar     *object_path,
+                      const gchar     *interface_name,
+                      const gchar     *signal_name,
+                      GVariant        *parameters,
+                      gpointer         user_data)
+{
+        const gchar *seat = NULL;
+        g_autoptr(GVariant) changed_props = NULL;
+        g_autoptr(GVariant) changed_prop = NULL;
+        g_autofree const gchar * const *invalidated_props = NULL;
+        gboolean changed = FALSE;
+        int res;
+
+        /* Extract seat id, i.e. the last element of the object path. */
+        seat = strrchr (object_path, '/');
+        if (seat == NULL)
+                return;
+        seat += 1;
+
+        g_variant_get (parameters, "(s@a{sv}^a&s)", NULL, &changed_props, &invalidated_props);
+
+        changed_prop = g_variant_lookup_value (changed_props, "CanGraphical", NULL);
+        if (changed_prop)
+                changed = TRUE;
+        if (!changed && g_strv_contains (invalidated_props, "CanGraphical"))
+                changed = TRUE;
+
+        if (!changed)
+                return;
+
+        res = sd_seat_can_graphical (seat);
+        if (res < 0)
+                return;
+
+        if (res)
+                create_display (GDM_LOCAL_DISPLAY_FACTORY (user_data), seat);
+        else
+                delete_display (GDM_LOCAL_DISPLAY_FACTORY (user_data), seat);
+}
+
 static gboolean
 lookup_by_session_id (const char *id,
                       GdmDisplay *display,
@@ -849,6 +942,16 @@ gdm_local_display_factory_start_monitor (GdmLocalDisplayFactory *factory)
                                                                              on_seat_removed,
                                                                              g_object_ref (factory),
                                                                              g_object_unref);
+        factory->seat_prop_changed_id = g_dbus_connection_signal_subscribe (factory->connection,
+                                                                            "org.freedesktop.login1",
+                                                                            
"org.freedesktop.DBus.Properties",
+                                                                            "PropertiesChanged",
+                                                                            NULL,
+                                                                            "org.freedesktop.login1.Seat",
+                                                                            
G_DBUS_SIGNAL_FLAGS_MATCH_ARG0_NAMESPACE,
+                                                                            on_seat_prop_changed,
+                                                                            g_object_ref (factory),
+                                                                            g_object_unref);
 
 #if defined(ENABLE_USER_DISPLAY_SERVER)
         io_channel = g_io_channel_new_file ("/sys/class/tty/tty0/active", "r", NULL);
@@ -877,6 +980,11 @@ gdm_local_display_factory_stop_monitor (GdmLocalDisplayFactory *factory)
                                                       factory->seat_removed_id);
                 factory->seat_removed_id = 0;
         }
+        if (factory->seat_prop_changed_id) {
+                g_dbus_connection_signal_unsubscribe (factory->connection,
+                                                      factory->seat_prop_changed_id);
+                factory->seat_prop_changed_id = 0;
+        }
 #if defined(ENABLE_USER_DISPLAY_SERVER)
         if (factory->active_vt_watch_id) {
                 g_source_remove (factory->active_vt_watch_id);
@@ -962,6 +1070,8 @@ gdm_local_display_factory_stop (GdmDisplayFactory *base_factory)
                                               G_CALLBACK (on_display_removed),
                                               factory);
 
+        g_clear_handle_id (&factory->force_graphical_timeout_id, g_source_remove);
+
         return TRUE;
 }