Merge branch 'settings-test-cleanups' into 'master'

Various memory leak cleanups to GSettings tests

See merge request GNOME/glib!610

10 files changed, 136 insertions, 34 deletions

diff --git a/gio/giomodule.c b/gio/giomodule.c
index 4c4c75b90..ca1daf2a8 100644
--- a/gio/giomodule.c
+++ b/gio/giomodule.c
@@ -893,6 +893,13 @@ try_implementation (const char           *extension_point,
     }
 }
 
+static void
+weak_ref_free (GWeakRef *weak_ref)
+{
+  g_weak_ref_clear (weak_ref);
+  g_free (weak_ref);
+}
+
 /**
  * _g_io_module_get_default:
  * @extension_point: the name of an extension point
@@ -914,10 +921,11 @@ try_implementation (const char           *extension_point,
  * be called on each candidate implementation after construction, to
  * check if it is actually usable or not.
  *
- * The result is cached after it is generated the first time, and
+ * The result is cached after it is generated the first time (but the cache does
+ * not keep a strong reference to the object), and
  * the function is thread-safe.
  *
- * Returns: (transfer none): an object implementing
+ * Returns: (transfer full) (nullable): an object implementing
  *     @extension_point, or %NULL if there are no usable
  *     implementations.
  */
@@ -932,25 +940,33 @@ _g_io_module_get_default (const gchar         *extension_point,
   GList *l;
   GIOExtensionPoint *ep;
   GIOExtension *extension = NULL, *preferred;
-  gpointer impl;
+  gpointer impl, value;
+  GWeakRef *impl_weak_ref = NULL;
 
   g_rec_mutex_lock (&default_modules_lock);
   if (default_modules)
     {
-      gpointer key;
-
       if (g_hash_table_lookup_extended (default_modules, extension_point,
-					&key, &impl))
-	{
+                                        NULL, &value))
+        {
           /* Don’t debug here, since we’re returning a cached object which was
            * already printed earlier. */
-	  g_rec_mutex_unlock (&default_modules_lock);
-	  return impl;
-	}
+          impl_weak_ref = value;
+          impl = g_weak_ref_get (impl_weak_ref);
+
+          /* If the object has been finalised (impl == NULL), fall through and
+           * instantiate a new one. */
+          if (impl != NULL)
+            {
+              g_rec_mutex_unlock (&default_modules_lock);
+              return g_steal_pointer (&impl);
+            }
+        }
     }
   else
     {
-      default_modules = g_hash_table_new (g_str_hash, g_str_equal);
+      default_modules = g_hash_table_new_full (g_str_hash, g_str_equal,
+                                               g_free, (GDestroyNotify) weak_ref_free);
     }
 
   _g_io_modules_ensure_loaded ();
@@ -1005,9 +1021,18 @@ _g_io_module_get_default (const gchar         *extension_point,
   impl = NULL;
 
  done:
-  g_hash_table_insert (default_modules,
-		       g_strdup (extension_point),
-		       impl ? g_object_ref (impl) : NULL);
+  if (impl_weak_ref == NULL)
+    {
+      impl_weak_ref = g_new0 (GWeakRef, 1);
+      g_weak_ref_init (impl_weak_ref, impl);
+      g_hash_table_insert (default_modules, g_strdup (extension_point),
+                           g_steal_pointer (&impl_weak_ref));
+    }
+  else
+    {
+      g_weak_ref_set (impl_weak_ref, impl);
+    }
+
   g_rec_mutex_unlock (&default_modules_lock);
 
   if (impl != NULL)
@@ -1021,7 +1046,7 @@ _g_io_module_get_default (const gchar         *extension_point,
     g_debug ("%s: Failed to find default implementation for ‘%s’",
              G_STRFUNC, extension_point);
 
-  return impl;
+  return g_steal_pointer (&impl);
 }
 
 G_LOCK_DEFINE_STATIC (registered_extensions);
diff --git a/gio/gnetworkmonitor.c b/gio/gnetworkmonitor.c
index 8027e4663..d32c60154 100644
--- a/gio/gnetworkmonitor.c
+++ b/gio/gnetworkmonitor.c
@@ -77,6 +77,7 @@ enum {
 };
 
 static guint signals[LAST_SIGNAL] = { 0 };
+static GNetworkMonitor *network_monitor_default_singleton = NULL;  /* (owned) (atomic) */
 
 /**
  * g_network_monitor_get_default:
@@ -91,9 +92,18 @@ static guint signals[LAST_SIGNAL] = { 0 };
 GNetworkMonitor *
 g_network_monitor_get_default (void)
 {
-  return _g_io_module_get_default (G_NETWORK_MONITOR_EXTENSION_POINT_NAME,
-                                   "GIO_USE_NETWORK_MONITOR",
-                                   NULL);
+  if (g_once_init_enter (&network_monitor_default_singleton))
+    {
+      GNetworkMonitor *singleton;
+
+      singleton = _g_io_module_get_default (G_NETWORK_MONITOR_EXTENSION_POINT_NAME,
+                                            "GIO_USE_NETWORK_MONITOR",
+                                            NULL);
+
+      g_once_init_leave (&network_monitor_default_singleton, singleton);
+    }
+
+  return network_monitor_default_singleton;
 }
 
 /**
diff --git a/gio/gproxyresolver.c b/gio/gproxyresolver.c
index c83347b52..0df51eb60 100644
--- a/gio/gproxyresolver.c
+++ b/gio/gproxyresolver.c
@@ -67,6 +67,8 @@ g_proxy_resolver_default_init (GProxyResolverInterface *iface)
 {
 }
 
+static GProxyResolver *proxy_resolver_default_singleton = NULL;  /* (owned) (atomic) */
+
 /**
  * g_proxy_resolver_get_default:
  *
@@ -80,9 +82,18 @@ g_proxy_resolver_default_init (GProxyResolverInterface *iface)
 GProxyResolver *
 g_proxy_resolver_get_default (void)
 {
-  return _g_io_module_get_default (G_PROXY_RESOLVER_EXTENSION_POINT_NAME,
-				   "GIO_USE_PROXY_RESOLVER",
-				   (GIOModuleVerifyFunc)g_proxy_resolver_is_supported);
+  if (g_once_init_enter (&proxy_resolver_default_singleton))
+    {
+      GProxyResolver *singleton;
+
+      singleton = _g_io_module_get_default (G_PROXY_RESOLVER_EXTENSION_POINT_NAME,
+                                            "GIO_USE_PROXY_RESOLVER",
+                                            (GIOModuleVerifyFunc) g_proxy_resolver_is_supported);
+
+      g_once_init_leave (&proxy_resolver_default_singleton, singleton);
+    }
+
+  return proxy_resolver_default_singleton;
 }
 
 /**
diff --git a/gio/gsettings.c b/gio/gsettings.c
index 4332ebe44..9a04a8630 100644
--- a/gio/gsettings.c
+++ b/gio/gsettings.c
@@ -3296,7 +3296,7 @@ g_settings_action_get_property (GObject *object, guint prop_id,
       break;
 
     case ACTION_PROP_STATE:
-      g_value_set_variant (value, g_settings_action_get_state (action));
+      g_value_take_variant (value, g_settings_action_get_state (action));
       break;
 
     default:
diff --git a/gio/gsettingsbackend.c b/gio/gsettingsbackend.c
index dcc7c3714..b124bc7ec 100644
--- a/gio/gsettingsbackend.c
+++ b/gio/gsettingsbackend.c
@@ -992,6 +992,12 @@ g_settings_backend_verify (gpointer impl)
   return TRUE;
 }
 
+/* We need to cache the default #GSettingsBackend for the entire process
+ * lifetime, especially if the backend is #GMemorySettingsBackend: it needs to
+ * keep the in-memory settings around even while there are no #GSettings
+ * instances alive. */
+static GSettingsBackend *settings_backend_default_singleton = NULL;  /* (owned) (atomic) */
+
 /**
  * g_settings_backend_get_default:
  *
@@ -1010,12 +1016,18 @@ g_settings_backend_verify (gpointer impl)
 GSettingsBackend *
 g_settings_backend_get_default (void)
 {
-  GSettingsBackend *backend;
+  if (g_once_init_enter (&settings_backend_default_singleton))
+    {
+      GSettingsBackend *singleton;
+
+      singleton = _g_io_module_get_default (G_SETTINGS_BACKEND_EXTENSION_POINT_NAME,
+                                            "GSETTINGS_BACKEND",
+                                            g_settings_backend_verify);
+
+      g_once_init_leave (&settings_backend_default_singleton, singleton);
+    }
 
-  backend = _g_io_module_get_default (G_SETTINGS_BACKEND_EXTENSION_POINT_NAME,
-				      "GSETTINGS_BACKEND",
-				      g_settings_backend_verify);
-  return g_object_ref (backend);
+  return g_object_ref (settings_backend_default_singleton);
 }
 
 /*< private >
diff --git a/gio/gtlsbackend.c b/gio/gtlsbackend.c
index 6d948adf1..25569aad7 100644
--- a/gio/gtlsbackend.c
+++ b/gio/gtlsbackend.c
@@ -93,6 +93,8 @@ g_tls_backend_default_init (GTlsBackendInterface *iface)
 {
 }
 
+static GTlsBackend *tls_backend_default_singleton = NULL;  /* (owned) (atomic) */
+
 /**
  * g_tls_backend_get_default:
  *
@@ -106,8 +108,18 @@ g_tls_backend_default_init (GTlsBackendInterface *iface)
 GTlsBackend *
 g_tls_backend_get_default (void)
 {
-  return _g_io_module_get_default (G_TLS_BACKEND_EXTENSION_POINT_NAME,
-				   "GIO_USE_TLS", NULL);
+  if (g_once_init_enter (&tls_backend_default_singleton))
+    {
+      GTlsBackend *singleton;
+
+      singleton = _g_io_module_get_default (G_TLS_BACKEND_EXTENSION_POINT_NAME,
+                                            "GIO_USE_TLS",
+                                            NULL);
+
+      g_once_init_leave (&tls_backend_default_singleton, singleton);
+    }
+
+  return tls_backend_default_singleton;
 }
 
 /**
diff --git a/gio/gvfs.c b/gio/gvfs.c
index f178d7a41..6e2dcf060 100644
--- a/gio/gvfs.c
+++ b/gio/gvfs.c
@@ -337,6 +337,8 @@ g_vfs_parse_name (GVfs       *vfs,
   return (* class->parse_name) (vfs, parse_name);
 }
 
+static GVfs *vfs_default_singleton = NULL;  /* (owned) (atomic) */
+
 /**
  * g_vfs_get_default:
  *
@@ -350,9 +352,19 @@ g_vfs_get_default (void)
 {
   if (GLIB_PRIVATE_CALL (g_check_setuid) ())
     return g_vfs_get_local ();
-  return _g_io_module_get_default (G_VFS_EXTENSION_POINT_NAME,
-                                   "GIO_USE_VFS",
-                                   (GIOModuleVerifyFunc)g_vfs_is_active);
+
+  if (g_once_init_enter (&vfs_default_singleton))
+    {
+      GVfs *singleton;
+
+      singleton = _g_io_module_get_default (G_VFS_EXTENSION_POINT_NAME,
+                                            "GIO_USE_VFS",
+                                            (GIOModuleVerifyFunc) g_vfs_is_active);
+
+      g_once_init_leave (&vfs_default_singleton, singleton);
+    }
+
+  return vfs_default_singleton;
 }
 
 /**
diff --git a/gio/tests/gsettings.c b/gio/tests/gsettings.c
index 179d0fd2f..16eb228c4 100644
--- a/gio/tests/gsettings.c
+++ b/gio/tests/gsettings.c
@@ -3001,5 +3001,19 @@ main (int argc, char *argv[])
 
   g_settings_sync ();
 
+  /* FIXME: Due to the way #GSettings objects can be used without specifying a
+   * backend, the default backend is leaked. In order to be able to run this
+   * test under valgrind and get meaningful checking for real leaks, use this
+   * hack to drop the final reference to the default #GSettingsBackend.
+   *
+   * This should not be used in production code. */
+    {
+      GSettingsBackend *backend;
+
+      backend = g_settings_backend_get_default ();
+      g_object_unref (backend);  /* reference from the *_get_default() call */
+      g_assert_finalize_object (backend);  /* singleton reference owned by GLib */
+    }
+
   return result;
 }
diff --git a/gio/tests/tls-bindings.c b/gio/tests/tls-bindings.c
index 89890248a..681b658c7 100644
--- a/gio/tests/tls-bindings.c
+++ b/gio/tests/tls-bindings.c
@@ -40,7 +40,6 @@ get_tls_channel_binding (void)
             G_TLS_CHANNEL_BINDING_TLS_UNIQUE, NULL, (GError **)&not_null));
 
   g_object_unref (tls);
-  g_object_unref (backend);
   g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_failed ();
   g_test_trap_assert_stderr ("*GLib-GIO-CRITICAL*");
@@ -76,7 +75,6 @@ get_dtls_channel_binding (void)
             G_TLS_CHANNEL_BINDING_TLS_UNIQUE, NULL, (GError **)&not_null));
 
   g_object_unref (dtls);
-  g_object_unref (backend);
   g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_failed ();
   g_test_trap_assert_stderr ("*GLib-GIO-CRITICAL*");
diff --git a/glib.supp b/glib.supp
index e89fd87fc..ab1b1f9dc 100644
--- a/glib.supp
+++ b/glib.supp
@@ -422,6 +422,14 @@
 }
 
 {
+	g-io-module-default-singleton-weak-ref
+	Memcheck:Leak
+	fun:calloc
+	...
+	fun:_g_io_module_get_default
+}
+
+{
 	g-get-language-names-malloc
 	Memcheck:Leak
 	match-leak-kinds:reachable