Blob Blame History Raw
From a9422c7b5f4200ad36300bb06134d545bb9d48d2 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Tue, 17 Jul 2018 20:20:55 +0000
Subject: [PATCH 01/51] 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 <stdlib.h>
 #include <stdio.h>
 
 #include <glib.h>
 #include <glib/gi18n.h>
 #include <glib-object.h>
 
 #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 ab7e12e91..1a9196ee1 100644
--- a/daemon/gdm-local-display-factory.c
+++ b/daemon/gdm-local-display-factory.c
@@ -222,107 +222,104 @@ 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_type = NULL;
         gboolean         is_initial = TRUE;
         gboolean         is_local = TRUE;
 
         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,
                       "is-initial", &is_initial,
                       "is-local", &is_local,
                       "session-type", &session_type,
                       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, do a full resync.  Only
                  * seats without displays will get created anyway.  This
                  * ensures we get a new login screen when the user logs out,
                  * if there isn't one.
                  */
                 if (is_local) {
                         /* reset num failures */
                         factory->priv->num_failures = 0;
 
                         gdm_local_display_factory_sync_seats (factory);
                 }
                 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;
 
                                         /* 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;
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.27.0