From b322fd907ef49a77a11ed6e5b9c858a7dd43d064 Mon Sep 17 00:00:00 2001

From: Lubomir Rintel <lkundrak@v3.sk>

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 <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 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