From d76d6ff0761d47df938f1dab0daeeecac2feb56e Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Mon, 6 Sep 2021 08:43:28 -0400 Subject: [PATCH 4/5] daemon: Consolidate session-type and supported-session-types list There's currently a bug in computing the session-type to use. The `i > 0` check means wayland will overwrite x11 in the transient session type list. Morever, the separation between "session-type" and "supported-session-types" is a little redundant. Since supported-session-types is a sorted list, the first item should always be the same as "session-type". This commit addresses the bug and the redundant logic, by computing the supported session types early in the function and indexing into it to get the session-type. A future cleanup could probably get rid of session-type entirely. https://gitlab.gnome.org/GNOME/gdm/-/merge_requests/153 --- daemon/gdm-local-display-factory.c | 193 +++++++++++++++++------------ 1 file changed, 116 insertions(+), 77 deletions(-) diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c index 141d64c6..eba38671 100644 --- a/daemon/gdm-local-display-factory.c +++ b/daemon/gdm-local-display-factory.c @@ -197,164 +197,226 @@ get_preferred_display_server (GdmLocalDisplayFactory *factory) } if (!wayland_enabled && !xorg_enabled) { return g_strdup ("none"); } gdm_settings_direct_get_string (GDM_KEY_PREFERRED_DISPLAY_SERVER, &preferred_display_server); if (g_strcmp0 (preferred_display_server, "wayland") == 0) { if (wayland_enabled) return g_strdup (preferred_display_server); else return g_strdup ("xorg"); } if (g_strcmp0 (preferred_display_server, "xorg") == 0) { if (xorg_enabled) return g_strdup (preferred_display_server); else return g_strdup ("wayland"); } if (g_strcmp0 (preferred_display_server, "legacy-xorg") == 0) { if (xorg_enabled) return g_strdup (preferred_display_server); } return g_strdup ("none"); } +struct GdmDisplayServerConfiguration { + const char *display_server; + const char *key; + const char *binary; + const char *session_type; +} display_server_configuration[] = { +#ifdef ENABLE_WAYLAND_SUPPORT + { "wayland", GDM_KEY_WAYLAND_ENABLE, "/usr/bin/Xwayland", "wayland" }, +#endif + { "xorg", GDM_KEY_XORG_ENABLE, "/usr/bin/Xorg", "x11" }, + { NULL, NULL, NULL }, +}; + +static gboolean +display_server_enabled (GdmLocalDisplayFactory *factory, + const char *display_server) +{ + size_t i; + + for (i = 0; display_server_configuration[i].display_server != NULL; i++) { + const char *key = display_server_configuration[i].key; + const char *binary = display_server_configuration[i].binary; + gboolean enabled = FALSE; + + if (!g_str_equal (display_server_configuration[i].display_server, + display_server)) + continue; + + if (!gdm_settings_direct_get_boolean (key, &enabled) || !enabled) + return FALSE; + + if (!g_file_test (binary, G_FILE_TEST_IS_EXECUTABLE)) + return FALSE; + + return TRUE; + } + + return FALSE; +} + static const char * -gdm_local_display_factory_get_session_type (GdmLocalDisplayFactory *factory, - gboolean should_fall_back) +get_session_type_for_display_server (GdmLocalDisplayFactory *factory, + const char *display_server) +{ + size_t i; + + for (i = 0; display_server_configuration[i].display_server != NULL; i++) { + if (!g_str_equal (display_server_configuration[i].display_server, + display_server)) + continue; + + return display_server_configuration[i].session_type; + } + + return NULL; +} + +static char ** +gdm_local_display_factory_get_session_types (GdmLocalDisplayFactory *factory, + gboolean should_fall_back) { - const char *session_types[3] = { NULL }; - gsize i, session_type_index = 0; g_autofree gchar *preferred_display_server = NULL; + const char *fallback_display_server = NULL; + gboolean wayland_preferred = FALSE; + gboolean xorg_preferred = FALSE; + g_autoptr (GPtrArray) session_types_array = NULL; + char **session_types; + + session_types_array = g_ptr_array_new (); preferred_display_server = get_preferred_display_server (factory); - if (g_strcmp0 (preferred_display_server, "wayland") != 0 && - g_strcmp0 (preferred_display_server, "xorg") != 0) - return NULL; + g_debug ("GdmLocalDisplayFactory: Getting session type (prefers %s, falling back: %s)", + preferred_display_server, should_fall_back? "yes" : "no"); - for (i = 0; i < G_N_ELEMENTS (session_types) - 1; i++) { -#ifdef ENABLE_WAYLAND_SUPPORT - if (i > 0 || - g_strcmp0 (preferred_display_server, "wayland") == 0) { - gboolean wayland_enabled = FALSE; - if (gdm_settings_direct_get_boolean (GDM_KEY_WAYLAND_ENABLE, &wayland_enabled)) { - if (wayland_enabled && g_file_test ("/usr/bin/Xwayland", G_FILE_TEST_IS_EXECUTABLE)) { - session_types[i] = "wayland"; - continue; - } - } - } -#endif + wayland_preferred = g_str_equal (preferred_display_server, "wayland"); + xorg_preferred = g_str_equal (preferred_display_server, "xorg"); + + if (wayland_preferred) + fallback_display_server = "xorg"; + else if (xorg_preferred) + fallback_display_server = "wayland"; + else + return NULL; - if (i > 0 || - g_strcmp0 (preferred_display_server, "xorg") == 0) { - gboolean xorg_enabled = FALSE; - if (gdm_settings_direct_get_boolean (GDM_KEY_XORG_ENABLE, &xorg_enabled)) { - if (xorg_enabled && g_file_test ("/usr/bin/Xorg", G_FILE_TEST_IS_EXECUTABLE)) { - session_types[i] = "x11"; - continue; - } - } - } + if (!should_fall_back) { + if (display_server_enabled (factory, preferred_display_server)) + g_ptr_array_add (session_types_array, (gpointer) get_session_type_for_display_server (factory, preferred_display_server)); } - if (should_fall_back) - session_type_index++; + if (display_server_enabled (factory, fallback_display_server)) + g_ptr_array_add (session_types_array, (gpointer) get_session_type_for_display_server (factory, fallback_display_server)); - return session_types[session_type_index]; + if (session_types_array->len == 0) + return NULL; + + g_ptr_array_add (session_types_array, NULL); + + session_types = g_strdupv ((char **) session_types_array->pdata); + + return session_types; } static void on_display_disposed (GdmLocalDisplayFactory *factory, GdmDisplay *display) { g_debug ("GdmLocalDisplayFactory: Display %p disposed", display); } static void store_display (GdmLocalDisplayFactory *factory, GdmDisplay *display) { GdmDisplayStore *store; store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory)); gdm_display_store_add (store, display); } /* Example: dbus-send --system --dest=org.gnome.DisplayManager \ --type=method_call --print-reply --reply-timeout=2000 \ /org/gnome/DisplayManager/Manager \ org.gnome.DisplayManager.Manager.GetDisplays */ gboolean gdm_local_display_factory_create_transient_display (GdmLocalDisplayFactory *factory, char **id, GError **error) { gboolean ret; GdmDisplay *display = NULL; gboolean is_initial = FALSE; const char *session_type; g_autofree gchar *preferred_display_server = NULL; g_return_val_if_fail (GDM_IS_LOCAL_DISPLAY_FACTORY (factory), FALSE); ret = FALSE; g_debug ("GdmLocalDisplayFactory: Creating transient display"); preferred_display_server = get_preferred_display_server (factory); #ifdef ENABLE_USER_DISPLAY_SERVER if (g_strcmp0 (preferred_display_server, "wayland") == 0 || g_strcmp0 (preferred_display_server, "xorg") == 0) { - session_type = gdm_local_display_factory_get_session_type (factory, FALSE); + g_auto(GStrv) session_types = NULL; - if (session_type == NULL) { + session_types = gdm_local_display_factory_get_session_types (factory, FALSE); + + if (session_types == NULL) { g_set_error_literal (error, GDM_DISPLAY_ERROR, GDM_DISPLAY_ERROR_GENERAL, "Both Wayland and Xorg are unavailable"); return FALSE; } display = gdm_local_display_new (); - g_object_set (G_OBJECT (display), "session-type", session_type, NULL); + g_object_set (G_OBJECT (display), + "session-type", session_types[0], + "supported-session-tyes", session_types, + NULL); is_initial = TRUE; } #endif if (g_strcmp0 (preferred_display_server, "legacy-xorg") == 0) { if (display == NULL) { guint32 num; num = take_next_display_number (factory); display = gdm_legacy_display_new (num); } } if (display == NULL) { g_set_error_literal (error, GDM_DISPLAY_ERROR, GDM_DISPLAY_ERROR_GENERAL, "Invalid preferred display server configured"); return FALSE; } g_object_set (display, "seat-id", "seat0", "allow-timed-login", FALSE, "is-initial", is_initial, NULL); store_display (factory, display); if (! gdm_display_manage (display)) { @@ -549,243 +611,220 @@ lookup_prepared_display_by_seat_id (const char *id, if (status != GDM_DISPLAY_PREPARED) return FALSE; return lookup_by_seat_id (id, display, user_data); } static int on_seat0_graphics_check_timeout (gpointer user_data) { GdmLocalDisplayFactory *factory = user_data; factory->seat0_graphics_check_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->seat0_graphics_check_timed_out = TRUE; ensure_display_for_seat (factory, "seat0"); return G_SOURCE_REMOVE; } static void ensure_display_for_seat (GdmLocalDisplayFactory *factory, const char *seat_id) { int ret; gboolean seat_supports_graphics; gboolean is_seat0; - const char *session_type = "wayland"; + g_auto (GStrv) session_types = NULL; + const char *legacy_session_types[] = { "x11", NULL }; GdmDisplayStore *store; GdmDisplay *display = NULL; g_autofree char *login_session_id = NULL; gboolean wayland_enabled = FALSE, xorg_enabled = FALSE; g_autofree gchar *preferred_display_server = NULL; - gboolean falling_back; + gboolean falling_back = FALSE; gdm_settings_direct_get_boolean (GDM_KEY_WAYLAND_ENABLE, &wayland_enabled); gdm_settings_direct_get_boolean (GDM_KEY_XORG_ENABLE, &xorg_enabled); preferred_display_server = get_preferred_display_server (factory); if (g_strcmp0 (preferred_display_server, "none") == 0) { g_debug ("GdmLocalDisplayFactory: Preferred display server is none, so not creating display"); return; } ret = sd_seat_can_graphical (seat_id); if (ret < 0) { g_critical ("Failed to query CanGraphical information for seat %s", seat_id); return; } if (ret == 0) { g_debug ("GdmLocalDisplayFactory: System doesn't currently support graphics"); seat_supports_graphics = FALSE; } else { g_debug ("GdmLocalDisplayFactory: System supports graphics"); seat_supports_graphics = TRUE; } if (g_strcmp0 (seat_id, "seat0") == 0) { is_seat0 = TRUE; falling_back = factory->num_failures > 0; - session_type = gdm_local_display_factory_get_session_type (factory, falling_back); + session_types = gdm_local_display_factory_get_session_types (factory, falling_back); g_debug ("GdmLocalDisplayFactory: New displays on seat0 will use %s%s", - session_type, falling_back? " fallback" : ""); + session_types[0], falling_back? " fallback" : ""); } else { is_seat0 = FALSE; g_debug ("GdmLocalDisplayFactory: New displays on seat %s will use X11 fallback", seat_id); /* Force legacy X11 for all auxiliary seats */ seat_supports_graphics = TRUE; - session_type = "x11"; + session_types = g_strdupv ((char **) legacy_session_types); } /* For seat0, we have a fallback logic to still try starting it after * SEAT0_GRAPHICS_CHECK_TIMEOUT seconds. i.e. we simply continue even if * CanGraphical is unset. * 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 wayland capable device is present and simply not marked as * master-of-seat. In these cases, this should likely be fixed in the * udev rules. * * At the moment, systemd always sets CanGraphical for non-seat0 seats. * This is because non-seat0 seats are defined by having master-of-seat * set. This means we can avoid the fallback check for non-seat0 seats, * which simplifies the code. */ if (is_seat0) { if (!seat_supports_graphics) { if (!factory->seat0_graphics_check_timed_out) { if (factory->seat0_graphics_check_timeout_id == 0) { g_debug ("GdmLocalDisplayFactory: seat0 doesn't yet support graphics. Waiting %d seconds to try again.", SEAT0_GRAPHICS_CHECK_TIMEOUT); factory->seat0_graphics_check_timeout_id = g_timeout_add_seconds (SEAT0_GRAPHICS_CHECK_TIMEOUT, on_seat0_graphics_check_timeout, factory); } else { /* It is not yet time to force X11 fallback. */ g_debug ("GdmLocalDisplayFactory: seat0 display requested when there is no graphics support before graphics check timeout."); } return; } g_debug ("GdmLocalDisplayFactory: Assuming we can use seat0 for X11 even though system says it doesn't support graphics!"); g_debug ("GdmLocalDisplayFactory: This might indicate an issue where the framebuffer device is not tagged as master-of-seat in udev."); seat_supports_graphics = TRUE; - session_type = "x11"; wayland_enabled = FALSE; + g_strfreev (session_types); + session_types = g_strdupv ((char **) legacy_session_types); } else { g_clear_handle_id (&factory->seat0_graphics_check_timeout_id, g_source_remove); } } if (!seat_supports_graphics) return; - if (session_type != NULL) + if (session_types != NULL) g_debug ("GdmLocalDisplayFactory: %s login display for seat %s requested", - session_type, seat_id); + session_types[0], seat_id); else if (g_strcmp0 (preferred_display_server, "legacy-xorg") == 0) g_debug ("GdmLocalDisplayFactory: Legacy Xorg login display for seat %s requested", seat_id); store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory)); if (is_seat0) display = gdm_display_store_find (store, lookup_prepared_display_by_seat_id, (gpointer) seat_id); else display = gdm_display_store_find (store, lookup_by_seat_id, (gpointer) seat_id); /* Ensure we don't create the same display more than once */ if (display != NULL) { g_debug ("GdmLocalDisplayFactory: display already created"); return; } /* If we already have a login window, switch to it */ if (gdm_get_login_window_session_id (seat_id, &login_session_id)) { GdmDisplay *display; display = gdm_display_store_find (store, lookup_by_session_id, (gpointer) login_session_id); if (display != NULL && (gdm_display_get_status (display) == GDM_DISPLAY_MANAGED || gdm_display_get_status (display) == GDM_DISPLAY_WAITING_TO_FINISH)) { g_object_set (G_OBJECT (display), "status", GDM_DISPLAY_MANAGED, NULL); g_debug ("GdmLocalDisplayFactory: session %s found, activating.", login_session_id); gdm_activate_session_by_id (factory->connection, seat_id, login_session_id); return; } } g_debug ("GdmLocalDisplayFactory: Adding display on seat %s", seat_id); #ifdef ENABLE_USER_DISPLAY_SERVER if (g_strcmp0 (preferred_display_server, "wayland") == 0 || g_strcmp0 (preferred_display_server, "xorg") == 0) { if (is_seat0) { - g_autoptr (GPtrArray) supported_session_types = NULL; - - if (session_type == NULL) { - g_warning ("GdmLocalDisplayFactory: Both Wayland and Xorg sessions are unavailable"); - return; - } - - supported_session_types = g_ptr_array_new (); - - if (g_strcmp0 (preferred_display_server, "wayland") == 0) { - if (wayland_enabled) - g_ptr_array_add (supported_session_types, "wayland"); - } else { - if (xorg_enabled) - g_ptr_array_add (supported_session_types, "x11"); - } - - if (!falling_back) { - if (g_strcmp0 (preferred_display_server, "wayland") == 0) { - if (xorg_enabled) - g_ptr_array_add (supported_session_types, "x11"); - } else { - if (wayland_enabled) - g_ptr_array_add (supported_session_types, "wayland"); - } - } - - g_ptr_array_add (supported_session_types, NULL); - display = gdm_local_display_new (); - g_object_set (G_OBJECT (display), "session-type", session_type, NULL); - g_object_set (G_OBJECT (display), "supported-session-types", supported_session_types->pdata, NULL); + g_object_set (G_OBJECT (display), + "session-type", session_types[0], + "supported-session-types", session_types, + NULL); } } #endif if (display == NULL) { guint32 num; - const char *supported_session_types[] = { "x11", NULL }; num = take_next_display_number (factory); display = gdm_legacy_display_new (num); - g_object_set (G_OBJECT (display), "supported-session-types", supported_session_types, NULL); + g_object_set (G_OBJECT (display), + "session-type", legacy_session_types[0], + "supported-session-types", legacy_session_types, + NULL); } g_object_set (display, "seat-id", seat_id, NULL); g_object_set (display, "is-initial", is_seat0, NULL); store_display (factory, display); /* let store own the ref */ g_object_unref (display); if (! gdm_display_manage (display)) { gdm_display_unmanage (display); } return; } static void delete_display (GdmLocalDisplayFactory *factory, const char *seat_id) { GdmDisplayStore *store; g_debug ("GdmLocalDisplayFactory: Removing used_display_numbers on seat %s", seat_id); store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory)); gdm_display_store_foreach_remove (store, lookup_by_seat_id, (gpointer) seat_id); } static gboolean -- 2.34.1