[gnome-settings-daemon] Bug 777255 - fontconfig update racy, causes update storm that freezes the desktop

[Behdad Esfahbod <behdad src gnome org>]

commit 45e5185dcaf1d26fa84ce367f89b7f720c80bd01
Author: Behdad Esfahbod <behdad behdad org>
Date:   Tue Aug 8 10:10:51 2017 -0700

    Bug 777255 - fontconfig update racy, causes update storm that freezes the desktop
    
    https://bugzilla.gnome.org/show_bug.cgi?id=777255
    
    Patch from Jan Alexander Steffens (heftig).

 plugins/xsettings/Makefile.am                      |    8 +-
 plugins/xsettings/fc-monitor.c                     |  317 ++++++++++++++++++++
 .../{fontconfig-monitor.h => fc-monitor.h}         |   23 +-
 plugins/xsettings/fontconfig-monitor.c             |  172 -----------
 plugins/xsettings/gsd-xsettings-manager.c          |   29 +-
 5 files changed, 344 insertions(+), 205 deletions(-)
---
diff --git a/plugins/xsettings/Makefile.am b/plugins/xsettings/Makefile.am
index 7420429..bc62c8a 100644
--- a/plugins/xsettings/Makefile.am
+++ b/plugins/xsettings/Makefile.am
@@ -25,8 +25,8 @@ test_gtk_modules_CPPFLAGS =                           \
 noinst_PROGRAMS += test-fontconfig-monitor
 
 test_fontconfig_monitor_SOURCES =      \
-       fontconfig-monitor.c            \
-       fontconfig-monitor.h
+       fc-monitor.c                    \
+       fc-monitor.h
 
 test_fontconfig_monitor_CFLAGS = $(PLUGIN_CFLAGS) $(XSETTINGS_CFLAGS)
 
@@ -61,8 +61,8 @@ gsd_xsettings_SOURCES =               \
        xsettings-common.h      \
        xsettings-manager.c     \
        xsettings-manager.h     \
-       fontconfig-monitor.c    \
-       fontconfig-monitor.h    \
+       fc-monitor.c    \
+       fc-monitor.h    \
        gsd-remote-display-manager.c \
        gsd-remote-display-manager.h \
        wm-button-layout-translation.c  \
diff --git a/plugins/xsettings/fc-monitor.c b/plugins/xsettings/fc-monitor.c
new file mode 100644
index 0000000..63e8712
--- /dev/null
+++ b/plugins/xsettings/fc-monitor.c
@@ -0,0 +1,317 @@
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 8 -*-
+ *
+ * Copyright (C) 2008 Red Hat, Inc.
+ * Copyright (C) 2017 Jan Alexander Steffens (heftig) <jan steffens gmail com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (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, see <http://www.gnu.org/licenses/>.
+ *
+ * Author:  Behdad Esfahbod, Red Hat, Inc.
+ */
+
+#include "fc-monitor.h"
+
+#include <gio/gio.h>
+#include <fontconfig/fontconfig.h>
+
+#define TIMEOUT_MILLISECONDS 1000
+
+static void
+fontconfig_cache_update_thread (GTask *task,
+                                gpointer source_object G_GNUC_UNUSED,
+                                gpointer task_data G_GNUC_UNUSED,
+                                GCancellable *cancellable G_GNUC_UNUSED)
+{
+        if (FcConfigUptoDate (NULL)) {
+                g_task_return_boolean (task, FALSE);
+                return;
+        }
+
+        if (!FcInitReinitialize ()) {
+                g_task_return_new_error (task, G_IO_ERROR, G_IO_ERROR_FAILED,
+                                         "FcInitReinitialize failed");
+                return;
+        }
+
+        g_task_return_boolean (task, TRUE);
+}
+
+static void
+fontconfig_cache_update_async (GAsyncReadyCallback callback,
+                               gpointer user_data)
+{
+        GTask *task = g_task_new (NULL, NULL, callback, user_data);
+        g_task_run_in_thread (task, fontconfig_cache_update_thread);
+        g_object_unref (task);
+}
+
+static gboolean
+fontconfig_cache_update_finish (GAsyncResult *result,
+                                GError **error)
+{
+        return g_task_propagate_boolean (G_TASK (result), error);
+}
+
+typedef enum {
+        UPDATE_IDLE,
+        UPDATE_PENDING,
+        UPDATE_RUNNING,
+        UPDATE_RESTART,
+} UpdateState;
+
+struct _FcMonitor {
+        GObject parent_instance;
+
+        GPtrArray *monitors;
+
+        guint timeout;
+        UpdateState state;
+        gboolean notify;
+};
+
+enum {
+        SIGNAL_UPDATED,
+
+        N_SIGNALS
+};
+
+static guint signals[N_SIGNALS] = { 0, };
+
+static void fc_monitor_finalize (GObject *object);
+static void monitor_files (FcMonitor *self, FcStrList *list);
+static void stuff_changed (GFileMonitor *monitor, GFile *file, GFile *other_file,
+                           GFileMonitorEvent event_type, gpointer data);
+static void start_timeout (FcMonitor *self);
+static gboolean start_update (gpointer data);
+static void update_done (GObject *source_object, GAsyncResult *result, gpointer user_data);
+
+G_DEFINE_TYPE (FcMonitor, fc_monitor, G_TYPE_OBJECT);
+
+static void
+fc_monitor_class_init (FcMonitorClass *klass)
+{
+        GObjectClass *object_class = G_OBJECT_CLASS (klass);
+
+        object_class->finalize = fc_monitor_finalize;
+
+        signals[SIGNAL_UPDATED] = g_signal_new ("updated",
+                                                G_TYPE_FROM_CLASS (klass),
+                                                G_SIGNAL_RUN_LAST,
+                                                0,
+                                                NULL,
+                                                NULL,
+                                                NULL,
+                                                G_TYPE_NONE,
+                                                0);
+}
+
+FcMonitor *
+fc_monitor_new (void)
+{
+        return g_object_new (FC_TYPE_MONITOR, NULL);
+}
+
+static void
+fc_monitor_init (FcMonitor *self G_GNUC_UNUSED)
+{
+        FcInit ();
+}
+
+static void
+fc_monitor_finalize (GObject *object)
+{
+        FcMonitor *self = FC_MONITOR (object);
+
+        if (self->timeout)
+                g_source_remove (self->timeout);
+        self->timeout = 0;
+
+        g_clear_pointer (&self->monitors, g_ptr_array_unref);
+
+        G_OBJECT_CLASS (fc_monitor_parent_class)->finalize (object);
+}
+
+void
+fc_monitor_start (FcMonitor *self)
+{
+        g_return_if_fail (FC_IS_MONITOR (self));
+        g_return_if_fail (self->monitors == NULL);
+
+        self->monitors = g_ptr_array_new_with_free_func (g_object_unref);
+
+        monitor_files (self, FcConfigGetConfigFiles (NULL));
+        monitor_files (self, FcConfigGetFontDirs (NULL));
+}
+
+void
+fc_monitor_stop (FcMonitor *self)
+{
+        g_return_if_fail (FC_IS_MONITOR (self));
+        g_clear_pointer (&self->monitors, g_ptr_array_unref);
+}
+
+static void
+monitor_files (FcMonitor *self,
+               FcStrList *list)
+{
+        const char *str;
+
+        while ((str = (const char *) FcStrListNext (list))) {
+                GFile *file;
+                GFileMonitor *monitor;
+
+                file = g_file_new_for_path (str);
+
+                monitor = g_file_monitor (file, G_FILE_MONITOR_NONE, NULL, NULL);
+
+                g_object_unref (file);
+
+                if (!monitor)
+                        continue;
+
+                g_signal_connect (monitor, "changed", G_CALLBACK (stuff_changed), self);
+
+                g_ptr_array_add (self->monitors, monitor);
+        }
+
+        FcStrListDone (list);
+}
+
+static const gchar *
+get_name (GType enum_type,
+          gint enum_value)
+{
+        GEnumClass *klass = g_type_class_ref (enum_type);
+        GEnumValue *value = g_enum_get_value (klass, enum_value);
+        const gchar *name = value ? value->value_name : "(unknown)";
+        g_type_class_unref (klass);
+        return name;
+}
+
+static void
+stuff_changed (GFileMonitor *monitor G_GNUC_UNUSED,
+               GFile *file G_GNUC_UNUSED,
+               GFile *other_file G_GNUC_UNUSED,
+               GFileMonitorEvent event_type,
+               gpointer data)
+{
+        FcMonitor *self = FC_MONITOR (data);
+        const gchar *event_name = get_name (G_TYPE_FILE_MONITOR_EVENT, event_type);
+
+        switch (self->state) {
+        case UPDATE_IDLE:
+                g_debug ("Got %-38s: starting fontconfig update timeout", event_name);
+                start_timeout (self);
+                break;
+
+        case UPDATE_PENDING:
+                /* wait for quiescence */
+                g_debug ("Got %-38s: restarting fontconfig update timeout", event_name);
+                g_source_remove (self->timeout);
+                start_timeout (self);
+                break;
+
+        case UPDATE_RUNNING:
+                g_debug ("Got %-38s: restarting fontconfig update", event_name);
+                self->state = UPDATE_RESTART;
+                break;
+
+        case UPDATE_RESTART:
+                g_debug ("Got %-38s: waiting on fontconfig update", event_name);
+                break;
+        }
+}
+
+static void
+start_timeout (FcMonitor *self)
+{
+        self->state = UPDATE_PENDING;
+        self->timeout = g_timeout_add (TIMEOUT_MILLISECONDS, start_update, self);
+        g_source_set_name_by_id (self->timeout, "[gnome-settings-daemon] update");
+}
+
+static gboolean
+start_update (gpointer data)
+{
+        FcMonitor *self = FC_MONITOR (data);
+
+        self->state = UPDATE_RUNNING;
+        self->timeout = 0;
+
+        g_debug ("Timeout completed: starting fontconfig update");
+        fontconfig_cache_update_async (update_done, g_object_ref (self));
+
+        return G_SOURCE_REMOVE;
+}
+
+static void
+update_done (GObject *source_object G_GNUC_UNUSED,
+             GAsyncResult *result,
+             gpointer data)
+{
+        FcMonitor *self = FC_MONITOR (data);
+        gboolean restart = self->state == UPDATE_RESTART;
+        GError *error = NULL;
+
+        self->state = UPDATE_IDLE;
+
+        if (fontconfig_cache_update_finish (result, &error)) {
+                g_debug ("Fontconfig update successful");
+                /* Remember we had a successful update even if we have to restart it */
+                self->notify = TRUE;
+        } else if (error) {
+                g_warning ("Fontconfig update failed: %s", error->message);
+                g_error_free (error);
+        } else
+                g_debug ("Fontconfig update was unnecessary");
+
+        if (restart) {
+                g_debug ("Concurrent change: restarting fontconfig update timeout");
+                start_timeout (self);
+        } else if (self->notify) {
+                self->notify = FALSE;
+
+                if (self->monitors) {
+                        fc_monitor_stop (self);
+                        fc_monitor_start (self);
+                }
+
+                /* we finish modifying self before emitting the signal,
+                 * allowing the callback to stop us if it decides to. */
+                g_signal_emit (self, signals[SIGNAL_UPDATED], 0);
+        }
+
+        /* release ref taken in start_update */
+        g_object_unref (self);
+}
+
+#ifdef FONTCONFIG_MONITOR_TEST
+static void
+yay (void)
+{
+        g_message ("yay");
+}
+
+int
+main (void)
+{
+        GMainLoop *loop = g_main_loop_new (NULL, TRUE);
+        FcMonitor *monitor = fc_monitor_new ();
+
+        fc_monitor_start (monitor);
+        g_signal_connect (monitor, "updated", G_CALLBACK (yay), NULL);
+
+        g_main_loop_run (loop);
+        return 0;
+}
+#endif
diff --git a/plugins/xsettings/fontconfig-monitor.h b/plugins/xsettings/fc-monitor.h
similarity index 58%
rename from plugins/xsettings/fontconfig-monitor.h
rename to plugins/xsettings/fc-monitor.h
index f3a92bb..4b564f8 100644
--- a/plugins/xsettings/fontconfig-monitor.h
+++ b/plugins/xsettings/fc-monitor.h
@@ -1,6 +1,6 @@
 /* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 8 -*-
  *
- * Copyright (C) 2008 Red Hat, Inc.
+ * Copyright (C) 2017 Jan Alexander Steffens (heftig) <jan steffens gmail com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -15,25 +15,22 @@
  * You should have received a copy of the GNU General Public License
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  *
- * Author:  Behdad Esfahbod, Red Hat, Inc.
  */
-#ifndef __FONTCONFIG_MONITOR_H
-#define __FONTCONFIG_MONITOR_H
+#ifndef FC_MONITOR_H
+#define FC_MONITOR_H
 
-#include <glib.h>
+#include <glib-object.h>
 
 G_BEGIN_DECLS
 
-void fontconfig_cache_init (void);
-gboolean fontconfig_cache_update (void);
+#define FC_TYPE_MONITOR (fc_monitor_get_type ())
+G_DECLARE_FINAL_TYPE (FcMonitor, fc_monitor, FC, MONITOR, GObject)
 
-typedef struct _fontconfig_monitor_handle fontconfig_monitor_handle_t;
+FcMonitor *fc_monitor_new (void);
 
-fontconfig_monitor_handle_t *
-fontconfig_monitor_start (GFunc    notify_callback,
-                          gpointer notify_data);
-void fontconfig_monitor_stop  (fontconfig_monitor_handle_t *handle);
+void fc_monitor_start (FcMonitor *monitor);
+void fc_monitor_stop  (FcMonitor *monitor);
 
 G_END_DECLS
 
-#endif /* __FONTCONFIG_MONITOR_H */
+#endif /* FC_MONITOR_H */
diff --git a/plugins/xsettings/gsd-xsettings-manager.c b/plugins/xsettings/gsd-xsettings-manager.c
index bba612f..ddbbe74 100644
--- a/plugins/xsettings/gsd-xsettings-manager.c
+++ b/plugins/xsettings/gsd-xsettings-manager.c
@@ -48,7 +48,7 @@
 #include "gsd-xsettings-manager.h"
 #include "gsd-xsettings-gtk.h"
 #include "xsettings-manager.h"
-#include "fontconfig-monitor.h"
+#include "fc-monitor.h"
 #include "gsd-remote-display-manager.h"
 #include "wm-button-layout-translation.h"
 
@@ -276,7 +276,7 @@ struct GnomeXSettingsManagerPrivate
         GHashTable        *settings;
 
         GSettings         *plugin_settings;
-        fontconfig_monitor_handle_t *fontconfig_handle;
+        FcMonitor         *fontconfig_monitor;
 
         GsdXSettingsGtk   *gtk;
 
@@ -1085,8 +1085,8 @@ gtk_modules_callback (GsdXSettingsGtk       *gtk,
 }
 
 static void
-fontconfig_callback (fontconfig_monitor_handle_t *handle,
-                     GnomeXSettingsManager       *manager)
+fontconfig_callback (FcMonitor              *monitor,
+                     GnomeXSettingsManager  *manager)
 {
         int timestamp = time (NULL);
 
@@ -1102,7 +1102,7 @@ start_fontconfig_monitor_idle_cb (GnomeXSettingsManager *manager)
 {
         gnome_settings_profile_start (NULL);
 
-        manager->priv->fontconfig_handle = fontconfig_monitor_start ((GFunc) fontconfig_callback, manager);
+        fc_monitor_start (manager->priv->fontconfig_monitor);
 
         gnome_settings_profile_end (NULL);
 
@@ -1116,7 +1116,8 @@ start_fontconfig_monitor (GnomeXSettingsManager  *manager)
 {
         gnome_settings_profile_start (NULL);
 
-        fontconfig_cache_init ();
+        manager->priv->fontconfig_monitor = fc_monitor_new ();
+        g_signal_connect (manager->priv->fontconfig_monitor, "updated", G_CALLBACK (fontconfig_callback), 
manager);
 
         manager->priv->start_idle_id = g_idle_add ((GSourceFunc) start_fontconfig_monitor_idle_cb, manager);
         g_source_set_name_by_id (manager->priv->start_idle_id, "[gnome-settings-daemon] 
start_fontconfig_monitor_idle_cb");
@@ -1125,15 +1126,6 @@ start_fontconfig_monitor (GnomeXSettingsManager  *manager)
 }
 
 static void
-stop_fontconfig_monitor (GnomeXSettingsManager  *manager)
-{
-        if (manager->priv->fontconfig_handle) {
-                fontconfig_monitor_stop (manager->priv->fontconfig_handle);
-                manager->priv->fontconfig_handle = NULL;
-        }
-}
-
-static void
 notify_have_shell (GnomeXSettingsManager   *manager,
                    gboolean                 have_shell)
 {
@@ -1501,7 +1493,12 @@ gnome_xsettings_manager_stop (GnomeXSettingsManager *manager)
                 p->plugin_settings = NULL;
         }
 
-        stop_fontconfig_monitor (manager);
+        if (p->fontconfig_monitor != NULL) {
+                g_signal_handlers_disconnect_by_data (p->fontconfig_monitor, manager);
+                fc_monitor_stop (p->fontconfig_monitor);
+                g_object_unref (p->fontconfig_monitor);
+                p->fontconfig_monitor = NULL;
+        }
 
         if (p->settings != NULL) {
                 g_hash_table_destroy (p->settings);