From b1d55f2c85203d92cecd0bc4112a78e7d8abc3a3 Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofourdan@redhat.com>
Date: Thu, 23 Jan 2020 14:25:29 +0100
Subject: [PATCH] gtksettings: Remove display from cache on closing

GTK caches the settings per display in a static `GArray`, keeping a
reference to the `GdkDisplay` as the key.

However, when closing the display, the corresponding entry is not
removed from the cache in `GtkSettings`.

So when reopening again a `GdkDisplay`, if the new address matches one
of the previously closed display, the cache will return the existing
`GtkSettings` from the cache, which still holds a reference to the old
`GdkScreen` which was freed along the `GdkDisplay`.

To avoid the issue, make sure to remove the `GdkDisplay` and
corresponding `GdkSettings` when closing the `GdkDisplay`.

Also, care must be taken not to recreate the `GdkSettings` and re-add
the `GdkDisplay` to the cache once the display is closed, and make sure
callers of `gtk_settings_get_for_display()` can deal with a returned
value being `NULL` if the display is closed.

Fixes: commit 360a3c16902 - "Use a cheaper way to store settings per
display"
---
 gtk/gtkicontheme.c |  7 ++++---
 gtk/gtkmodules.c   |  8 ++++----
 gtk/gtksettings.c  | 31 ++++++++++++++++++++++++++++++-
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/gtk/gtkicontheme.c b/gtk/gtkicontheme.c
index 33634254a2..179078eec8 100644
--- a/gtk/gtkicontheme.c
+++ b/gtk/gtkicontheme.c
@@ -608,9 +608,10 @@ unset_screen (GtkIconTheme *icon_theme)
       g_signal_handlers_disconnect_by_func (display,
                                             (gpointer) display_closed,
                                             icon_theme);
-      g_signal_handlers_disconnect_by_func (settings,
-                                            (gpointer) theme_changed,
-                                            icon_theme);
+      if (settings)
+        g_signal_handlers_disconnect_by_func (settings,
+                                              (gpointer) theme_changed,
+                                              icon_theme);
 
       priv->screen = NULL;
     }
diff --git a/gtk/gtkmodules.c b/gtk/gtkmodules.c
index 0aaedae581..704e412aeb 100644
--- a/gtk/gtkmodules.c
+++ b/gtk/gtkmodules.c
@@ -463,10 +463,10 @@ display_closed_cb (GdkDisplay *display,
 
   screen = gdk_display_get_default_screen (display);
   settings = gtk_settings_get_for_screen (screen);
-
-  g_object_set_data_full (G_OBJECT (settings),
-			  I_("gtk-modules"),
-			  NULL, NULL);
+  if (settings)
+    g_object_set_data_full (G_OBJECT (settings),
+			    I_("gtk-modules"),
+			    NULL, NULL);
 }
 		   
 
diff --git a/gtk/gtksettings.c b/gtk/gtksettings.c
index b786385127..71a1c29902 100644
--- a/gtk/gtksettings.c
+++ b/gtk/gtksettings.c
@@ -1900,6 +1900,29 @@ settings_init_style (GtkSettings *settings)
   settings_update_key_theme (settings);
 }
 
+static void
+settings_display_closed (GdkDisplay *display,
+                         gboolean    is_error,
+                         gpointer    data)
+{
+  DisplaySettings *ds;
+  int i;
+
+  if (G_UNLIKELY (display_settings == NULL))
+    return;
+
+  ds = (DisplaySettings *)display_settings->data;
+  for (i = 0; i < display_settings->len; i++)
+    {
+      if (ds[i].display == display)
+        {
+          g_clear_object (&ds[i].settings);
+          display_settings = g_array_remove_index_fast (display_settings, i);
+          break;
+        }
+    }
+}
+
 static GtkSettings *
 gtk_settings_create_for_display (GdkDisplay *display)
 {
@@ -1955,7 +1978,9 @@ gtk_settings_create_for_display (GdkDisplay *display)
 
   v.display = display;
   v.settings = settings;
-  g_array_append_val (display_settings, v);
+  display_settings = g_array_append_val (display_settings, v);
+  g_signal_connect (display, "closed",
+                    G_CALLBACK (settings_display_closed), NULL);
 
   settings_init_style (settings);
   settings_update_xsettings (settings);
@@ -1975,6 +2000,10 @@ gtk_settings_get_for_display (GdkDisplay *display)
   DisplaySettings *ds;
   int i;
 
+  /* If the display is closed, we don't want to recreate the settings! */
+  if G_UNLIKELY (gdk_display_is_closed (display))
+    return NULL;
+
   if G_UNLIKELY (display_settings == NULL)
     display_settings = g_array_new (FALSE, TRUE, sizeof (DisplaySettings));