Commit 799e0242 authored by Allison Karlitskaya's avatar Allison Karlitskaya

improve thread safety in GDelayedSettingsBackend

  - hold a lock while accessing the tree of delayed values
  - use weak reference counts with the owner object to avoid doing
    g_object_notify on a dead object
  - dispatch the "has-unapplied" notify to the proper main context
parent 61219e26
...@@ -31,7 +31,10 @@ ...@@ -31,7 +31,10 @@
struct _GDelayedSettingsBackendPrivate struct _GDelayedSettingsBackendPrivate
{ {
GSettingsBackend *backend; GSettingsBackend *backend;
GStaticMutex lock;
GTree *delayed; GTree *delayed;
GMainContext *owner_context;
gpointer owner; gpointer owner;
}; };
...@@ -39,6 +42,53 @@ G_DEFINE_TYPE (GDelayedSettingsBackend, ...@@ -39,6 +42,53 @@ G_DEFINE_TYPE (GDelayedSettingsBackend,
g_delayed_settings_backend, g_delayed_settings_backend,
G_TYPE_SETTINGS_BACKEND) G_TYPE_SETTINGS_BACKEND)
static gboolean
invoke_notify_unapplied (gpointer data)
{
g_object_notify (data, "has-unapplied");
g_object_unref (data);
return FALSE;
}
static void
g_delayed_settings_backend_notify_unapplied (GDelayedSettingsBackend *delayed)
{
GMainContext *target_context;
GObject *target;
g_static_mutex_lock (&delayed->priv->lock);
if (delayed->priv->owner)
{
target_context = delayed->priv->owner_context;
target = g_object_ref (delayed->priv->owner);
}
else
{
target_context = NULL;
target = NULL;
}
g_static_mutex_unlock (&delayed->priv->lock);
if (target != NULL)
{
if (g_settings_backend_get_active_context () != target_context)
{
GSource *source;
source = g_idle_source_new ();
g_source_set_priority (source, G_PRIORITY_DEFAULT);
g_source_set_callback (source, invoke_notify_unapplied,
target, g_object_unref);
g_source_attach (source, target_context);
g_source_unref (source);
}
else
invoke_notify_unapplied (target);
}
}
static GVariant * static GVariant *
g_delayed_settings_backend_read (GSettingsBackend *backend, g_delayed_settings_backend_read (GSettingsBackend *backend,
const gchar *key, const gchar *key,
...@@ -64,13 +114,16 @@ g_delayed_settings_backend_write (GSettingsBackend *backend, ...@@ -64,13 +114,16 @@ g_delayed_settings_backend_write (GSettingsBackend *backend,
GDelayedSettingsBackend *delayed = G_DELAYED_SETTINGS_BACKEND (backend); GDelayedSettingsBackend *delayed = G_DELAYED_SETTINGS_BACKEND (backend);
gboolean was_empty; gboolean was_empty;
g_static_mutex_lock (&delayed->priv->lock);
was_empty = g_tree_nnodes (delayed->priv->delayed) == 0; was_empty = g_tree_nnodes (delayed->priv->delayed) == 0;
g_tree_insert (delayed->priv->delayed, g_strdup (key), g_tree_insert (delayed->priv->delayed, g_strdup (key),
g_variant_ref_sink (value)); g_variant_ref_sink (value));
g_static_mutex_unlock (&delayed->priv->lock);
g_settings_backend_changed (backend, key, origin_tag); g_settings_backend_changed (backend, key, origin_tag);
if (was_empty && delayed->priv->owner) if (was_empty)
g_object_notify (delayed->priv->owner, "has-unapplied"); g_delayed_settings_backend_notify_unapplied (delayed);
return TRUE; return TRUE;
} }
...@@ -92,14 +145,16 @@ g_delayed_settings_backend_write_keys (GSettingsBackend *backend, ...@@ -92,14 +145,16 @@ g_delayed_settings_backend_write_keys (GSettingsBackend *backend,
GDelayedSettingsBackend *delayed = G_DELAYED_SETTINGS_BACKEND (backend); GDelayedSettingsBackend *delayed = G_DELAYED_SETTINGS_BACKEND (backend);
gboolean was_empty; gboolean was_empty;
g_static_mutex_lock (&delayed->priv->lock);
was_empty = g_tree_nnodes (delayed->priv->delayed) == 0; was_empty = g_tree_nnodes (delayed->priv->delayed) == 0;
g_tree_foreach (tree, add_to_tree, delayed->priv->delayed); g_tree_foreach (tree, add_to_tree, delayed->priv->delayed);
g_static_mutex_unlock (&delayed->priv->lock);
g_settings_backend_changed_tree (backend, tree, origin_tag); g_settings_backend_changed_tree (backend, tree, origin_tag);
if (was_empty && delayed->priv->owner) if (was_empty)
g_object_notify (delayed->priv->owner, "has-unapplied"); g_delayed_settings_backend_notify_unapplied (delayed);
return TRUE; return TRUE;
} }
...@@ -147,11 +202,12 @@ g_delayed_settings_backend_unsubscribe (GSettingsBackend *backend, ...@@ -147,11 +202,12 @@ g_delayed_settings_backend_unsubscribe (GSettingsBackend *backend,
g_settings_backend_unsubscribe (delayed->priv->backend, name); g_settings_backend_unsubscribe (delayed->priv->backend, name);
} }
/* method calls */ /* method calls */
gboolean gboolean
g_delayed_settings_backend_get_has_unapplied (GDelayedSettingsBackend *delayed) g_delayed_settings_backend_get_has_unapplied (GDelayedSettingsBackend *delayed)
{ {
/* we don't need to lock for this... */
return g_tree_nnodes (delayed->priv->delayed) > 0; return g_tree_nnodes (delayed->priv->delayed) > 0;
} }
...@@ -163,10 +219,12 @@ g_delayed_settings_backend_apply (GDelayedSettingsBackend *delayed) ...@@ -163,10 +219,12 @@ g_delayed_settings_backend_apply (GDelayedSettingsBackend *delayed)
gboolean success; gboolean success;
GTree *tmp; GTree *tmp;
g_static_mutex_lock (&delayed->priv->lock);
tmp = delayed->priv->delayed; tmp = delayed->priv->delayed;
delayed->priv->delayed = g_settings_backend_create_tree (); delayed->priv->delayed = g_settings_backend_create_tree ();
success = g_settings_backend_write_keys (delayed->priv->backend, success = g_settings_backend_write_keys (delayed->priv->backend,
tmp, delayed->priv); tmp, delayed->priv);
g_static_mutex_unlock (&delayed->priv->lock);
if (!success) if (!success)
g_settings_backend_changed_tree (G_SETTINGS_BACKEND (delayed), g_settings_backend_changed_tree (G_SETTINGS_BACKEND (delayed),
...@@ -174,8 +232,7 @@ g_delayed_settings_backend_apply (GDelayedSettingsBackend *delayed) ...@@ -174,8 +232,7 @@ g_delayed_settings_backend_apply (GDelayedSettingsBackend *delayed)
g_tree_unref (tmp); g_tree_unref (tmp);
if (delayed->priv->owner) g_delayed_settings_backend_notify_unapplied (delayed);
g_object_notify (delayed->priv->owner, "has-unapplied");
} }
} }
...@@ -186,13 +243,14 @@ g_delayed_settings_backend_revert (GDelayedSettingsBackend *delayed) ...@@ -186,13 +243,14 @@ g_delayed_settings_backend_revert (GDelayedSettingsBackend *delayed)
{ {
GTree *tmp; GTree *tmp;
g_static_mutex_lock (&delayed->priv->lock);
tmp = delayed->priv->delayed; tmp = delayed->priv->delayed;
delayed->priv->delayed = g_settings_backend_create_tree (); delayed->priv->delayed = g_settings_backend_create_tree ();
g_static_mutex_unlock (&delayed->priv->lock);
g_settings_backend_changed_tree (G_SETTINGS_BACKEND (delayed), tmp, NULL); g_settings_backend_changed_tree (G_SETTINGS_BACKEND (delayed), tmp, NULL);
g_tree_unref (tmp); g_tree_unref (tmp);
if (delayed->priv->owner) g_delayed_settings_backend_notify_unapplied (delayed);
g_object_notify (delayed->priv->owner, "has-unapplied");
} }
} }
...@@ -243,21 +301,27 @@ delayed_backend_writable_changed (GSettingsBackend *backend, ...@@ -243,21 +301,27 @@ delayed_backend_writable_changed (GSettingsBackend *backend,
const gchar *key) const gchar *key)
{ {
GDelayedSettingsBackend *delayed = G_DELAYED_SETTINGS_BACKEND (target); GDelayedSettingsBackend *delayed = G_DELAYED_SETTINGS_BACKEND (target);
gboolean last_one = FALSE;
g_static_mutex_lock (&delayed->priv->lock);
if (g_tree_lookup (delayed->priv->delayed, key) && if (g_tree_lookup (delayed->priv->delayed, key) &&
!g_settings_backend_get_writable (delayed->priv->backend, key)) !g_settings_backend_get_writable (delayed->priv->backend, key))
{ {
/* drop the key from our changeset if it just became read-only. /* drop the key from our changeset if it just became read-only.
* no need to signal this since the writable change implies it. * no need to signal since the writable change below implies it.
*/ */
g_tree_remove (delayed->priv->delayed, key); g_tree_remove (delayed->priv->delayed, key);
/* if that was the only key... */ /* if that was the only key... */
if (delayed->priv->owner && last_one = g_tree_nnodes (delayed->priv->delayed) == 0;
g_tree_nnodes (delayed->priv->delayed) == 0)
g_object_notify (delayed->priv->owner, "has-unapplied");
} }
g_static_mutex_unlock (&delayed->priv->lock);
if (last_one)
g_delayed_settings_backend_notify_unapplied (delayed);
g_settings_backend_writable_changed (G_SETTINGS_BACKEND (delayed), key); g_settings_backend_writable_changed (G_SETTINGS_BACKEND (delayed), key);
} }
...@@ -289,8 +353,11 @@ delayed_backend_path_writable_changed (GSettingsBackend *backend, ...@@ -289,8 +353,11 @@ delayed_backend_path_writable_changed (GSettingsBackend *backend,
const gchar *path) const gchar *path)
{ {
GDelayedSettingsBackend *delayed = G_DELAYED_SETTINGS_BACKEND (target); GDelayedSettingsBackend *delayed = G_DELAYED_SETTINGS_BACKEND (target);
gboolean last_one = FALSE;
gsize n_keys; gsize n_keys;
g_static_mutex_lock (&delayed->priv->lock);
n_keys = g_tree_nnodes (delayed->priv->delayed); n_keys = g_tree_nnodes (delayed->priv->delayed);
if (n_keys > 0) if (n_keys > 0)
...@@ -309,11 +376,14 @@ delayed_backend_path_writable_changed (GSettingsBackend *backend, ...@@ -309,11 +376,14 @@ delayed_backend_path_writable_changed (GSettingsBackend *backend,
g_free (state.keys); g_free (state.keys);
if (delayed->priv->owner && last_one = g_tree_nnodes (delayed->priv->delayed) == 0;
g_tree_nnodes (delayed->priv->delayed) == 0)
g_object_notify (delayed->priv->owner, "has-unapplied");
} }
g_static_mutex_unlock (&delayed->priv->lock);
if (last_one)
g_delayed_settings_backend_notify_unapplied (delayed);
g_settings_backend_path_writable_changed (G_SETTINGS_BACKEND (delayed), g_settings_backend_path_writable_changed (G_SETTINGS_BACKEND (delayed),
path); path);
} }
...@@ -323,7 +393,14 @@ g_delayed_settings_backend_finalize (GObject *object) ...@@ -323,7 +393,14 @@ g_delayed_settings_backend_finalize (GObject *object)
{ {
GDelayedSettingsBackend *delayed = G_DELAYED_SETTINGS_BACKEND (object); GDelayedSettingsBackend *delayed = G_DELAYED_SETTINGS_BACKEND (object);
g_static_mutex_free (&delayed->priv->lock);
g_object_unref (delayed->priv->backend); g_object_unref (delayed->priv->backend);
/* if our owner is still alive, why are we finalizing? */
g_assert (delayed->priv->owner == NULL);
G_OBJECT_CLASS (g_delayed_settings_backend_parent_class)
->finalize (object);
} }
static void static void
...@@ -354,18 +431,35 @@ g_delayed_settings_backend_init (GDelayedSettingsBackend *delayed) ...@@ -354,18 +431,35 @@ g_delayed_settings_backend_init (GDelayedSettingsBackend *delayed)
GDelayedSettingsBackendPrivate); GDelayedSettingsBackendPrivate);
delayed->priv->delayed = g_settings_backend_create_tree (); delayed->priv->delayed = g_settings_backend_create_tree ();
g_static_mutex_init (&delayed->priv->lock);
}
static void
g_delayed_settings_backend_disown (gpointer data,
GObject *where_the_object_was)
{
GDelayedSettingsBackend *delayed = data;
g_static_mutex_lock (&delayed->priv->lock);
delayed->priv->owner_context = NULL;
delayed->priv->owner = NULL;
g_static_mutex_unlock (&delayed->priv->lock);
} }
GDelayedSettingsBackend * GDelayedSettingsBackend *
g_delayed_settings_backend_new (GSettingsBackend *backend, g_delayed_settings_backend_new (GSettingsBackend *backend,
gpointer owner) gpointer owner,
GMainContext *owner_context)
{ {
GDelayedSettingsBackend *delayed; GDelayedSettingsBackend *delayed;
delayed = g_object_new (G_TYPE_DELAYED_SETTINGS_BACKEND, NULL); delayed = g_object_new (G_TYPE_DELAYED_SETTINGS_BACKEND, NULL);
delayed->priv->backend = g_object_ref (backend); delayed->priv->backend = g_object_ref (backend);
delayed->priv->owner_context = owner_context;
delayed->priv->owner = owner; delayed->priv->owner = owner;
g_object_weak_ref (owner, g_delayed_settings_backend_disown, delayed);
g_settings_backend_watch (delayed->priv->backend, G_OBJECT (delayed), NULL, g_settings_backend_watch (delayed->priv->backend, G_OBJECT (delayed), NULL,
delayed_backend_changed, delayed_backend_changed,
delayed_backend_path_changed, delayed_backend_path_changed,
......
...@@ -60,9 +60,8 @@ G_GNUC_INTERNAL ...@@ -60,9 +60,8 @@ G_GNUC_INTERNAL
GType g_delayed_settings_backend_get_type (void); GType g_delayed_settings_backend_get_type (void);
G_GNUC_INTERNAL G_GNUC_INTERNAL
GDelayedSettingsBackend * g_delayed_settings_backend_new (GSettingsBackend *backend, GDelayedSettingsBackend * g_delayed_settings_backend_new (GSettingsBackend *backend,
gpointer owner); gpointer owner,
G_GNUC_INTERNAL GMainContext *owner_context);
void g_delayed_settings_backend_disown (GDelayedSettingsBackend *backend);
G_GNUC_INTERNAL G_GNUC_INTERNAL
void g_delayed_settings_backend_revert (GDelayedSettingsBackend *delayed); void g_delayed_settings_backend_revert (GDelayedSettingsBackend *delayed);
G_GNUC_INTERNAL G_GNUC_INTERNAL
......
...@@ -410,7 +410,9 @@ g_settings_delay (GSettings *settings) ...@@ -410,7 +410,9 @@ g_settings_delay (GSettings *settings)
return; return;
settings->priv->delayed = settings->priv->delayed =
g_delayed_settings_backend_new (settings->priv->backend, settings); g_delayed_settings_backend_new (settings->priv->backend,
settings,
settings->priv->main_context);
g_settings_backend_unwatch (settings->priv->backend, G_OBJECT (settings)); g_settings_backend_unwatch (settings->priv->backend, G_OBJECT (settings));
g_object_unref (settings->priv->backend); g_object_unref (settings->priv->backend);
......
...@@ -126,7 +126,7 @@ is_path (const gchar *path) ...@@ -126,7 +126,7 @@ is_path (const gchar *path)
return TRUE; return TRUE;
} }
static GMainContext * GMainContext *
g_settings_backend_get_active_context (void) g_settings_backend_get_active_context (void)
{ {
GMainContext *context; GMainContext *context;
...@@ -177,7 +177,7 @@ struct _GSettingsBackendClosure ...@@ -177,7 +177,7 @@ struct _GSettingsBackendClosure
static void static void
g_settings_backend_watch_weak_notify (gpointer data, g_settings_backend_watch_weak_notify (gpointer data,
GObject *where_object_was) GObject *where_the_object_was)
{ {
GSettingsBackend *backend = data; GSettingsBackend *backend = data;
GSettingsBackendWatch **ptr; GSettingsBackendWatch **ptr;
...@@ -185,7 +185,7 @@ g_settings_backend_watch_weak_notify (gpointer data, ...@@ -185,7 +185,7 @@ g_settings_backend_watch_weak_notify (gpointer data,
/* search and remove */ /* search and remove */
g_static_mutex_lock (&backend->priv->lock); g_static_mutex_lock (&backend->priv->lock);
for (ptr = &backend->priv->watches; *ptr; ptr = &(*ptr)->next) for (ptr = &backend->priv->watches; *ptr; ptr = &(*ptr)->next)
if ((*ptr)->target == where_object_was) if ((*ptr)->target == where_the_object_was)
{ {
GSettingsBackendWatch *tmp = *ptr; GSettingsBackendWatch *tmp = *ptr;
......
...@@ -98,5 +98,7 @@ void g_settings_backend_unsubscribe (GSettin ...@@ -98,5 +98,7 @@ void g_settings_backend_unsubscribe (GSettin
G_GNUC_INTERNAL G_GNUC_INTERNAL
void g_settings_backend_subscribe (GSettingsBackend *backend, void g_settings_backend_subscribe (GSettingsBackend *backend,
const char *name); const char *name);
G_GNUC_INTERNAL
GMainContext * g_settings_backend_get_active_context (void);
#endif /* __G_SETTINGS_BACKEND_INTERNAL_H__ */ #endif /* __G_SETTINGS_BACKEND_INTERNAL_H__ */
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment