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

Various memory leak cleanups to GSettings tests

See merge request GNOME/glib!610
This commit is contained in:
Philip Withnall 2021-01-20 13:15:26 +00:00
commit b6a1fa47fe
10 changed files with 136 additions and 34 deletions

View File

@ -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: * _g_io_module_get_default:
* @extension_point: the name of an extension point * @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 * be called on each candidate implementation after construction, to
* check if it is actually usable or not. * 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. * 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 * @extension_point, or %NULL if there are no usable
* implementations. * implementations.
*/ */
@ -932,25 +940,33 @@ _g_io_module_get_default (const gchar *extension_point,
GList *l; GList *l;
GIOExtensionPoint *ep; GIOExtensionPoint *ep;
GIOExtension *extension = NULL, *preferred; GIOExtension *extension = NULL, *preferred;
gpointer impl; gpointer impl, value;
GWeakRef *impl_weak_ref = NULL;
g_rec_mutex_lock (&default_modules_lock); g_rec_mutex_lock (&default_modules_lock);
if (default_modules) if (default_modules)
{ {
gpointer key;
if (g_hash_table_lookup_extended (default_modules, extension_point, if (g_hash_table_lookup_extended (default_modules, extension_point,
&key, &impl)) NULL, &value))
{ {
/* Dont debug here, since were returning a cached object which was /* Dont debug here, since were returning a cached object which was
* already printed earlier. */ * already printed earlier. */
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); g_rec_mutex_unlock (&default_modules_lock);
return impl; return g_steal_pointer (&impl);
}
} }
} }
else 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 (); _g_io_modules_ensure_loaded ();
@ -1005,9 +1021,18 @@ _g_io_module_get_default (const gchar *extension_point,
impl = NULL; impl = NULL;
done: done:
g_hash_table_insert (default_modules, if (impl_weak_ref == NULL)
g_strdup (extension_point), {
impl ? g_object_ref (impl) : 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); g_rec_mutex_unlock (&default_modules_lock);
if (impl != NULL) 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_debug ("%s: Failed to find default implementation for %s",
G_STRFUNC, extension_point); G_STRFUNC, extension_point);
return impl; return g_steal_pointer (&impl);
} }
G_LOCK_DEFINE_STATIC (registered_extensions); G_LOCK_DEFINE_STATIC (registered_extensions);

View File

@ -77,6 +77,7 @@ enum {
}; };
static guint signals[LAST_SIGNAL] = { 0 }; static guint signals[LAST_SIGNAL] = { 0 };
static GNetworkMonitor *network_monitor_default_singleton = NULL; /* (owned) (atomic) */
/** /**
* g_network_monitor_get_default: * g_network_monitor_get_default:
@ -91,9 +92,18 @@ static guint signals[LAST_SIGNAL] = { 0 };
GNetworkMonitor * GNetworkMonitor *
g_network_monitor_get_default (void) g_network_monitor_get_default (void)
{ {
return _g_io_module_get_default (G_NETWORK_MONITOR_EXTENSION_POINT_NAME, 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", "GIO_USE_NETWORK_MONITOR",
NULL); NULL);
g_once_init_leave (&network_monitor_default_singleton, singleton);
}
return network_monitor_default_singleton;
} }
/** /**

View File

@ -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: * g_proxy_resolver_get_default:
* *
@ -80,9 +82,18 @@ g_proxy_resolver_default_init (GProxyResolverInterface *iface)
GProxyResolver * GProxyResolver *
g_proxy_resolver_get_default (void) g_proxy_resolver_get_default (void)
{ {
return _g_io_module_get_default (G_PROXY_RESOLVER_EXTENSION_POINT_NAME, 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", "GIO_USE_PROXY_RESOLVER",
(GIOModuleVerifyFunc) g_proxy_resolver_is_supported); (GIOModuleVerifyFunc) g_proxy_resolver_is_supported);
g_once_init_leave (&proxy_resolver_default_singleton, singleton);
}
return proxy_resolver_default_singleton;
} }
/** /**

View File

@ -3296,7 +3296,7 @@ g_settings_action_get_property (GObject *object, guint prop_id,
break; break;
case ACTION_PROP_STATE: 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; break;
default: default:

View File

@ -992,6 +992,12 @@ g_settings_backend_verify (gpointer impl)
return TRUE; 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: * g_settings_backend_get_default:
* *
@ -1010,12 +1016,18 @@ g_settings_backend_verify (gpointer impl)
GSettingsBackend * GSettingsBackend *
g_settings_backend_get_default (void) g_settings_backend_get_default (void)
{ {
GSettingsBackend *backend; if (g_once_init_enter (&settings_backend_default_singleton))
{
GSettingsBackend *singleton;
backend = _g_io_module_get_default (G_SETTINGS_BACKEND_EXTENSION_POINT_NAME, singleton = _g_io_module_get_default (G_SETTINGS_BACKEND_EXTENSION_POINT_NAME,
"GSETTINGS_BACKEND", "GSETTINGS_BACKEND",
g_settings_backend_verify); g_settings_backend_verify);
return g_object_ref (backend);
g_once_init_leave (&settings_backend_default_singleton, singleton);
}
return g_object_ref (settings_backend_default_singleton);
} }
/*< private > /*< private >

View File

@ -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: * g_tls_backend_get_default:
* *
@ -106,8 +108,18 @@ g_tls_backend_default_init (GTlsBackendInterface *iface)
GTlsBackend * GTlsBackend *
g_tls_backend_get_default (void) g_tls_backend_get_default (void)
{ {
return _g_io_module_get_default (G_TLS_BACKEND_EXTENSION_POINT_NAME, if (g_once_init_enter (&tls_backend_default_singleton))
"GIO_USE_TLS", NULL); {
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;
} }
/** /**

View File

@ -337,6 +337,8 @@ g_vfs_parse_name (GVfs *vfs,
return (* class->parse_name) (vfs, parse_name); return (* class->parse_name) (vfs, parse_name);
} }
static GVfs *vfs_default_singleton = NULL; /* (owned) (atomic) */
/** /**
* g_vfs_get_default: * g_vfs_get_default:
* *
@ -350,9 +352,19 @@ g_vfs_get_default (void)
{ {
if (GLIB_PRIVATE_CALL (g_check_setuid) ()) if (GLIB_PRIVATE_CALL (g_check_setuid) ())
return g_vfs_get_local (); return g_vfs_get_local ();
return _g_io_module_get_default (G_VFS_EXTENSION_POINT_NAME,
if (g_once_init_enter (&vfs_default_singleton))
{
GVfs *singleton;
singleton = _g_io_module_get_default (G_VFS_EXTENSION_POINT_NAME,
"GIO_USE_VFS", "GIO_USE_VFS",
(GIOModuleVerifyFunc) g_vfs_is_active); (GIOModuleVerifyFunc) g_vfs_is_active);
g_once_init_leave (&vfs_default_singleton, singleton);
}
return vfs_default_singleton;
} }
/** /**

View File

@ -3001,5 +3001,19 @@ main (int argc, char *argv[])
g_settings_sync (); 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; return result;
} }

View File

@ -40,7 +40,6 @@ get_tls_channel_binding (void)
G_TLS_CHANNEL_BINDING_TLS_UNIQUE, NULL, (GError **)&not_null)); G_TLS_CHANNEL_BINDING_TLS_UNIQUE, NULL, (GError **)&not_null));
g_object_unref (tls); g_object_unref (tls);
g_object_unref (backend);
g_test_trap_subprocess (NULL, 0, 0); g_test_trap_subprocess (NULL, 0, 0);
g_test_trap_assert_failed (); g_test_trap_assert_failed ();
g_test_trap_assert_stderr ("*GLib-GIO-CRITICAL*"); 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_TLS_CHANNEL_BINDING_TLS_UNIQUE, NULL, (GError **)&not_null));
g_object_unref (dtls); g_object_unref (dtls);
g_object_unref (backend);
g_test_trap_subprocess (NULL, 0, 0); g_test_trap_subprocess (NULL, 0, 0);
g_test_trap_assert_failed (); g_test_trap_assert_failed ();
g_test_trap_assert_stderr ("*GLib-GIO-CRITICAL*"); g_test_trap_assert_stderr ("*GLib-GIO-CRITICAL*");

View File

@ -421,6 +421,14 @@
fun:_g_io_module_get_default* fun:_g_io_module_get_default*
} }
{
g-io-module-default-singleton-weak-ref
Memcheck:Leak
fun:calloc
...
fun:_g_io_module_get_default
}
{ {
g-get-language-names-malloc g-get-language-names-malloc
Memcheck:Leak Memcheck:Leak