[epiphany/mcatanzaro/#865] Use only two GKeyfileSettingsBackends per process



commit fe45246f2ed5e55261d83c0609078ecd6b55f861
Author: Michael Catanzaro <mcatanzaro igalia com>
Date:   Wed Jul 24 18:52:39 2019 -0500

    Use only two GKeyfileSettingsBackends per process
    
    Garnacho says that using a different GKeyfileSettingsBackend for each
    GSettings instance is causing Epiphany to consume thousands of inotify
    handles. Uh-oh. This is unnecessary so let's just avoid it.
    
    Probably fixes #865

 lib/ephy-permissions-manager.c | 20 ++++++++++++--------
 lib/ephy-settings.c            | 17 ++++++++++-------
 2 files changed, 22 insertions(+), 15 deletions(-)
---
diff --git a/lib/ephy-permissions-manager.c b/lib/ephy-permissions-manager.c
index bd7e88dcb..030a08390 100644
--- a/lib/ephy-permissions-manager.c
+++ b/lib/ephy-permissions-manager.c
@@ -39,6 +39,8 @@ struct _EphyPermissionsManager {
 
   GHashTable *permission_type_permitted_origins;
   GHashTable *permission_type_denied_origins;
+
+  GSettingsBackend *backend;
 };
 
 G_DEFINE_TYPE (EphyPermissionsManager, ephy_permissions_manager, G_TYPE_OBJECT)
@@ -48,6 +50,8 @@ G_DEFINE_TYPE (EphyPermissionsManager, ephy_permissions_manager, G_TYPE_OBJECT)
 static void
 ephy_permissions_manager_init (EphyPermissionsManager *manager)
 {
+  g_autofree char *filename = NULL;
+
   manager->origins_mapping = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_object_unref);
   manager->settings_mapping = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_free);
 
@@ -55,6 +59,11 @@ ephy_permissions_manager_init (EphyPermissionsManager *manager)
    * the GList keys without destroying the contents of the lists. */
   manager->permission_type_permitted_origins = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, 
NULL);
   manager->permission_type_denied_origins = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, 
NULL);
+
+  /* The GKeyfileBackend needs to be shared to avoid overconsumption of inotify
+   * handles. See: epiphany #865. */
+  filename = g_build_filename (ephy_profile_dir (), PERMISSIONS_FILENAME, NULL);
+  manager->backend = g_keyfile_settings_backend_new (filename, "/", NULL);
 }
 
 static void
@@ -85,6 +94,8 @@ ephy_permissions_manager_dispose (GObject *object)
     manager->permission_type_denied_origins = NULL;
   }
 
+  g_clear_object (&manager->backend);
+
   G_OBJECT_CLASS (ephy_permissions_manager_parent_class)->dispose (object);
 }
 
@@ -102,8 +113,6 @@ ephy_permissions_manager_get_settings_for_origin (EphyPermissionsManager *manage
 {
   char *origin_path;
   char *trimmed_protocol;
-  char *filename;
-  GSettingsBackend *backend;
   GSettings *settings;
   WebKitSecurityOrigin *security_origin;
   char *pos;
@@ -114,10 +123,6 @@ ephy_permissions_manager_get_settings_for_origin (EphyPermissionsManager *manage
   if (settings)
     return settings;
 
-  filename = g_build_filename (ephy_profile_dir (), PERMISSIONS_FILENAME, NULL);
-  backend = g_keyfile_settings_backend_new (filename, "/", NULL);
-  g_free (filename);
-
   /* Cannot contain consecutive slashes in GSettings path... */
   security_origin = webkit_security_origin_new_for_uri (origin);
   trimmed_protocol = g_strdup (webkit_security_origin_get_protocol (security_origin));
@@ -130,10 +135,9 @@ ephy_permissions_manager_get_settings_for_origin (EphyPermissionsManager *manage
                                  webkit_security_origin_get_host (security_origin),
                                  webkit_security_origin_get_port (security_origin));
 
-  settings = g_settings_new_with_backend_and_path ("org.gnome.Epiphany.permissions", backend, origin_path);
+  settings = g_settings_new_with_backend_and_path ("org.gnome.Epiphany.permissions", manager->backend, 
origin_path);
   g_free (trimmed_protocol);
   g_free (origin_path);
-  g_object_unref (backend);
   webkit_security_origin_unref (security_origin);
 
   /* Note that settings is owned only by the first hash table! */
diff --git a/lib/ephy-settings.c b/lib/ephy-settings.c
index 2d29b5618..b2b568c8d 100644
--- a/lib/ephy-settings.c
+++ b/lib/ephy-settings.c
@@ -204,7 +204,7 @@ ephy_settings_get_for_web_process_extension (const char *schema)
   gsettings = g_hash_table_lookup (settings, key_name);
 
   if (gsettings == NULL) {
-    g_autoptr (GSettingsBackend) backend = NULL;
+    static GSettingsBackend *backend = NULL;
     g_autoptr (GSettings) web_gsettings = NULL;
     g_autofree char *keyfile_path = NULL;
     g_autofree char *path = NULL;
@@ -212,17 +212,20 @@ ephy_settings_get_for_web_process_extension (const char *schema)
     gsettings = ephy_settings_get (schema);
     g_assert (gsettings != NULL);
 
-    /* GLib inside Flatpak will default to this backend in the future */
-    /* so we don't need to do anything extra */
+    /* GLib inside Flatpak already defaults to this backend. */
     g_object_get (gsettings, "backend", &backend, NULL);
-    /* G_IS_KEYFILE_SETTINGS_BACKEND () is private API */
-    if (!g_strcmp0 (g_type_name (G_TYPE_FROM_INSTANCE (backend)), "GKeyfileSettingsBackend")) {
+    if (g_strcmp0 (g_type_name (G_TYPE_FROM_INSTANCE (backend)), "GKeyfileSettingsBackend") == 0) {
       g_hash_table_insert (settings, g_steal_pointer (&key_name), g_object_ref (gsettings));
       return gsettings;
     }
 
-    keyfile_path = g_build_filename (ephy_config_dir (), "web-extension-settings.ini", NULL);
-    backend = g_keyfile_settings_backend_new (keyfile_path, "/", "/");
+    if (!backend) {
+      /* The GKeyfileBackend needs to be shared to avoid overconsumption of
+       * inotify handles. See: epiphany #865.
+       */
+      keyfile_path = g_build_filename (ephy_config_dir (), "web-extension-settings.ini", NULL);
+      backend = g_keyfile_settings_backend_new (keyfile_path, "/", "/");
+    }
 
     path = get_relocatable_path (schema);
     if (path != NULL)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]