From b322fd907ef49a77a11ed6e5b9c858a7dd43d064 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 17 Jul 2018 20:20:55 +0000 Subject: [PATCH 5/7] display-factory: avoid removing a display from store while iterating it --- daemon/gdm-display-factory.c | 41 ++++++++++++++++++++++++++++++ daemon/gdm-display-factory.h | 1 + daemon/gdm-local-display-factory.c | 7 ++--- daemon/gdm-xdmcp-display-factory.c | 7 ++--- 4 files changed, 46 insertions(+), 10 deletions(-) diff --git a/daemon/gdm-display-factory.c b/daemon/gdm-display-factory.c index d86a4c8ad..c520e1088 100644 --- a/daemon/gdm-display-factory.c +++ b/daemon/gdm-display-factory.c @@ -8,84 +8,120 @@ * (at your option) any later version. * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * */ #include "config.h" #include #include #include #include #include #include "gdm-display-factory.h" #include "gdm-display-store.h" #define GDM_DISPLAY_FACTORY_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), GDM_TYPE_DISPLAY_FACTORY, GdmDisplayFactoryPrivate)) struct GdmDisplayFactoryPrivate { GdmDisplayStore *display_store; + guint purge_displays_id; }; enum { PROP_0, PROP_DISPLAY_STORE, }; static void gdm_display_factory_class_init (GdmDisplayFactoryClass *klass); static void gdm_display_factory_init (GdmDisplayFactory *factory); static void gdm_display_factory_finalize (GObject *object); G_DEFINE_ABSTRACT_TYPE (GdmDisplayFactory, gdm_display_factory, G_TYPE_OBJECT) GQuark gdm_display_factory_error_quark (void) { static GQuark ret = 0; if (ret == 0) { ret = g_quark_from_static_string ("gdm_display_factory_error"); } return ret; } +static gboolean +purge_display (char *id, + GdmDisplay *display, + gpointer user_data) +{ + int status; + + status = gdm_display_get_status (display); + + switch (status) { + case GDM_DISPLAY_FINISHED: + case GDM_DISPLAY_FAILED: + return TRUE; + default: + return FALSE; + } +} + +static void +purge_displays (GdmDisplayFactory *factory) +{ + factory->priv->purge_displays_id = 0; + gdm_display_store_foreach_remove (factory->priv->display_store, + (GdmDisplayStoreFunc)purge_display, + NULL); +} + +void +gdm_display_factory_queue_purge_displays (GdmDisplayFactory *factory) +{ + if (factory->priv->purge_displays_id == 0) { + factory->priv->purge_displays_id = g_idle_add ((GSourceFunc) purge_displays, factory); + } +} + GdmDisplayStore * gdm_display_factory_get_display_store (GdmDisplayFactory *factory) { g_return_val_if_fail (GDM_IS_DISPLAY_FACTORY (factory), NULL); return factory->priv->display_store; } gboolean gdm_display_factory_start (GdmDisplayFactory *factory) { gboolean ret; g_return_val_if_fail (GDM_IS_DISPLAY_FACTORY (factory), FALSE); g_object_ref (factory); ret = GDM_DISPLAY_FACTORY_GET_CLASS (factory)->start (factory); g_object_unref (factory); return ret; } gboolean gdm_display_factory_stop (GdmDisplayFactory *factory) { gboolean ret; g_return_val_if_fail (GDM_IS_DISPLAY_FACTORY (factory), FALSE); g_object_ref (factory); @@ -160,32 +196,37 @@ gdm_display_factory_class_init (GdmDisplayFactoryClass *klass) g_object_class_install_property (object_class, PROP_DISPLAY_STORE, g_param_spec_object ("display-store", "display store", "display store", GDM_TYPE_DISPLAY_STORE, G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY)); g_type_class_add_private (klass, sizeof (GdmDisplayFactoryPrivate)); } static void gdm_display_factory_init (GdmDisplayFactory *factory) { factory->priv = GDM_DISPLAY_FACTORY_GET_PRIVATE (factory); } static void gdm_display_factory_finalize (GObject *object) { GdmDisplayFactory *factory; g_return_if_fail (object != NULL); g_return_if_fail (GDM_IS_DISPLAY_FACTORY (object)); factory = GDM_DISPLAY_FACTORY (object); g_return_if_fail (factory->priv != NULL); + if (factory->priv->purge_displays_id != 0) { + g_source_remove (factory->priv->purge_displays_id); + factory->priv->purge_displays_id = 0; + } + G_OBJECT_CLASS (gdm_display_factory_parent_class)->finalize (object); } diff --git a/daemon/gdm-display-factory.h b/daemon/gdm-display-factory.h index 6b30f83dc..1cffa1bd5 100644 --- a/daemon/gdm-display-factory.h +++ b/daemon/gdm-display-factory.h @@ -37,34 +37,35 @@ G_BEGIN_DECLS typedef struct GdmDisplayFactoryPrivate GdmDisplayFactoryPrivate; typedef struct { GObject parent; GdmDisplayFactoryPrivate *priv; } GdmDisplayFactory; typedef struct { GObjectClass parent_class; gboolean (*start) (GdmDisplayFactory *factory); gboolean (*stop) (GdmDisplayFactory *factory); } GdmDisplayFactoryClass; typedef enum { GDM_DISPLAY_FACTORY_ERROR_GENERAL } GdmDisplayFactoryError; #define GDM_DISPLAY_FACTORY_ERROR gdm_display_factory_error_quark () GQuark gdm_display_factory_error_quark (void); GType gdm_display_factory_get_type (void); gboolean gdm_display_factory_start (GdmDisplayFactory *manager); gboolean gdm_display_factory_stop (GdmDisplayFactory *manager); GdmDisplayStore * gdm_display_factory_get_display_store (GdmDisplayFactory *manager); +void gdm_display_factory_queue_purge_displays (GdmDisplayFactory *manager); G_END_DECLS #endif /* __GDM_DISPLAY_FACTORY_H */ diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c index 6856d30d0..1c7daeb14 100644 --- a/daemon/gdm-local-display-factory.c +++ b/daemon/gdm-local-display-factory.c @@ -224,128 +224,125 @@ gdm_local_display_factory_create_transient_display (GdmLocalDisplayFactory *fact "seat-id", "seat0", "allow-timed-login", FALSE, NULL); store_display (factory, display); if (! gdm_display_manage (display)) { display = NULL; goto out; } if (! gdm_display_get_id (display, id, NULL)) { display = NULL; goto out; } ret = TRUE; out: /* ref either held by store or not at all */ g_object_unref (display); return ret; } static void on_display_status_changed (GdmDisplay *display, GParamSpec *arg1, GdmLocalDisplayFactory *factory) { int status; - GdmDisplayStore *store; int num; char *seat_id = NULL; char *session_id = NULL; char *session_type = NULL; char *session_class = NULL; gboolean is_initial = TRUE; gboolean is_local = TRUE; int ret; num = -1; gdm_display_get_x11_display_number (display, &num, NULL); - store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory)); - g_object_get (display, "seat-id", &seat_id, "session-id", &session_id, "is-initial", &is_initial, "is-local", &is_local, "session-type", &session_type, "session-class", &session_class, NULL); status = gdm_display_get_status (display); g_debug ("GdmLocalDisplayFactory: display status changed: %d", status); switch (status) { case GDM_DISPLAY_FINISHED: /* remove the display number from factory->priv->used_display_numbers so that it may be reused */ if (num != -1) { g_hash_table_remove (factory->priv->used_display_numbers, GUINT_TO_POINTER (num)); } - gdm_display_store_remove (store, display); + gdm_display_factory_queue_purge_displays (GDM_DISPLAY_FACTORY (factory)); /* if this is a local display, recreate the display so * a new login screen comes up if one is missing. */ if (is_local && g_strcmp0 (session_class, "greeter") != 0) { g_autofree char *active_session = NULL; /* reset num failures */ factory->priv->num_failures = 0; ret = sd_seat_get_active (seat_id, &active_session, NULL); if (ret == 0) { g_autofree char *state = NULL; ret = sd_session_get_state (active_session, &state); if (ret != 0 || g_strcmp0 (state, "closing") == 0 || g_strcmp0 (active_session, session_id) == 0) { g_clear_pointer (&active_session, free); } } /* If this died in the foreground leaving us on a blank vt, start a new login screen */ if (!sd_seat_can_multi_session (seat_id) || active_session == NULL) { create_display (factory, seat_id, session_type, is_initial); } } break; case GDM_DISPLAY_FAILED: /* leave the display number in factory->priv->used_display_numbers so that it doesn't get reused */ - gdm_display_store_remove (store, display); + gdm_display_factory_queue_purge_displays (GDM_DISPLAY_FACTORY (factory)); /* 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; } #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: break; default: g_assert_not_reached (); break; diff --git a/daemon/gdm-xdmcp-display-factory.c b/daemon/gdm-xdmcp-display-factory.c index 46a0d9ffa..5b5786c6f 100644 --- a/daemon/gdm-xdmcp-display-factory.c +++ b/daemon/gdm-xdmcp-display-factory.c @@ -2039,93 +2039,90 @@ on_hostname_selected (GdmXdmcpChooserDisplay *display, char *ip; ic->chosen_address = gdm_address_new_from_sockaddr (ai->ai_addr, ai->ai_addrlen); ip = NULL; gdm_address_get_numeric_info (ic->chosen_address, &ip, NULL); g_debug ("GdmXdmcpDisplayFactory: hostname resolves to %s", ip ? ip : "(null)"); g_free (ip); } freeaddrinfo (ai_list); } static void on_client_disconnected (GdmDisplay *display) { if (gdm_display_get_status (display) != GDM_DISPLAY_MANAGED) return; gdm_display_stop_greeter_session (display); gdm_display_unmanage (display); gdm_display_finish (display); } static void on_display_status_changed (GdmDisplay *display, GParamSpec *arg1, GdmXdmcpDisplayFactory *factory) { int status; - GdmDisplayStore *store; GdmLaunchEnvironment *launch_environment; GdmSession *session; GdmAddress *address; gint32 session_number; int display_number; - store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory)); - launch_environment = NULL; g_object_get (display, "launch-environment", &launch_environment, NULL); session = NULL; if (launch_environment != NULL) { session = gdm_launch_environment_get_session (launch_environment); } status = gdm_display_get_status (display); g_debug ("GdmXdmcpDisplayFactory: xdmcp display status changed: %d", status); switch (status) { case GDM_DISPLAY_FINISHED: g_object_get (display, "remote-address", &address, "x11-display-number", &display_number, "session-number", &session_number, NULL); gdm_xdmcp_send_alive (factory, address, display_number, session_number); - gdm_display_store_remove (store, display); + gdm_display_factory_queue_purge_displays (GDM_DISPLAY_FACTORY (factory)); break; case GDM_DISPLAY_FAILED: - gdm_display_store_remove (store, display); + gdm_display_factory_queue_purge_displays (GDM_DISPLAY_FACTORY (factory)); break; case GDM_DISPLAY_UNMANAGED: if (session != NULL) { g_signal_handlers_disconnect_by_func (G_OBJECT (session), G_CALLBACK (on_client_disconnected), display); } break; case GDM_DISPLAY_PREPARED: break; case GDM_DISPLAY_MANAGED: if (session != NULL) { g_signal_connect_object (G_OBJECT (session), "client-disconnected", G_CALLBACK (on_client_disconnected), display, G_CONNECT_SWAPPED); g_signal_connect_object (G_OBJECT (session), "disconnected", G_CALLBACK (on_client_disconnected), display, G_CONNECT_SWAPPED); } break; default: g_assert_not_reached (); break; } } static GdmDisplay * gdm_xdmcp_display_create (GdmXdmcpDisplayFactory *factory, -- 2.21.0