diff options
| -rw-r--r-- | client/dconf-client.c | 26 | ||||
| -rw-r--r-- | engine/dconf-engine.c | 153 | ||||
| -rw-r--r-- | engine/dconf-engine.h | 5 | ||||
| -rw-r--r-- | gsettings/dconfsettingsbackend.c | 37 |
4 files changed, 199 insertions, 22 deletions
diff --git a/client/dconf-client.c b/client/dconf-client.c index ffc179e..1e3efc0 100644 --- a/client/dconf-client.c +++ b/client/dconf-client.c @@ -47,7 +47,7 @@ dconf_client_finalize (GObject *object) { DConfClient *client = DCONF_CLIENT (object); - dconf_engine_free (client->engine); + dconf_engine_unref (client->engine); g_main_context_unref (client->context); G_OBJECT_CLASS (dconf_client_parent_class) @@ -103,13 +103,19 @@ dconf_engine_change_notify (DConfEngine *engine, const gchar * tag, gpointer user_data) { - DConfClient *client = user_data; + GWeakRef *weak_ref = user_data; DConfClientChange *change; + DConfClient *client; + + client = g_weak_ref_get (weak_ref); + + if (client == NULL) + return; g_return_if_fail (DCONF_IS_CLIENT (client)); change = g_slice_new (DConfClientChange); - change->client = g_object_ref (client); + change->client = client; change->prefix = g_strdup (prefix); change->changes = g_strdupv ((gchar **) changes); change->tag = g_strdup (tag); @@ -117,13 +123,25 @@ dconf_engine_change_notify (DConfEngine *engine, g_main_context_invoke (client->context, dconf_client_dispatch_change_signal, change); } +static void +dconf_client_free_weak_ref (gpointer data) +{ + GWeakRef *weak_ref = data; + + g_weak_ref_clear (weak_ref); + g_slice_free (GWeakRef, weak_ref); +} + DConfClient * dconf_client_new (void) { DConfClient *client; + GWeakRef *weak_ref; client = g_object_new (DCONF_TYPE_CLIENT, NULL); - client->engine = dconf_engine_new (client); + weak_ref = g_slice_new (GWeakRef); + g_weak_ref_init (weak_ref, client); + client->engine = dconf_engine_new (weak_ref, dconf_client_free_weak_ref); client->context = g_main_context_ref_thread_default (); return client; diff --git a/engine/dconf-engine.c b/engine/dconf-engine.c index 230d2ab..75d39e0 100644 --- a/engine/dconf-engine.c +++ b/engine/dconf-engine.c @@ -147,18 +147,24 @@ #define MAX_IN_FLIGHT 2 +static GSList *dconf_engine_global_list; +static GMutex dconf_engine_global_lock; + struct _DConfEngine { gpointer user_data; /* Set at construct time */ + GDestroyNotify free_func; + gint ref_count; GMutex sources_lock; /* This lock is for the sources (ie: refreshing) and state. */ guint64 state; /* Counter that changes every time a source is refreshed. */ DConfEngineSource **sources; /* Array never changes, but each source changes internally. */ gint n_sources; - GMutex queue_lock; /* This lock is for pending, in_flight, last_handled */ + GMutex queue_lock; /* This lock is for pending, in_flight */ GQueue pending; /* DConfChangeset */ GQueue in_flight; /* DConfChangeset */ + gchar *last_handled; /* reply tag from last item in in_flight */ }; @@ -214,13 +220,16 @@ dconf_engine_unlock_queues (DConfEngine *engine) } DConfEngine * -dconf_engine_new (gpointer user_data) +dconf_engine_new (gpointer user_data, + GDestroyNotify free_func) { DConfEngine *engine; gint i; engine = g_slice_new0 (DConfEngine); engine->user_data = user_data; + engine->free_func = free_func; + engine->ref_count = 1; g_mutex_init (&engine->sources_lock); g_mutex_init (&engine->queue_lock); @@ -231,23 +240,83 @@ dconf_engine_new (gpointer user_data) if (!dconf_engine_source_init (engine->sources[i])) g_assert_not_reached (); + g_mutex_lock (&dconf_engine_global_lock); + dconf_engine_global_list = g_slist_prepend (dconf_engine_global_list, engine); + g_mutex_unlock (&dconf_engine_global_lock); + return engine; } void -dconf_engine_free (DConfEngine *engine) +dconf_engine_unref (DConfEngine *engine) { - gint i; + if (g_atomic_int_dec_and_test (&engine->ref_count)) + { + gint i; - g_mutex_clear (&engine->sources_lock); - g_mutex_clear (&engine->queue_lock); + /* We just dropped our refcount to zero, but we're still in the + * dconf_engine_global_list. + * + * If a signal arrives at this exact instant and the signal + * handler beats us to the lock then the refcount will be + * increased again. + * + * Acquire the lock and then double-check that the refcount is + * still zero before actually doing the remove. If it's non-zero + * then the signal handler grabbed a ref and will call unref() + * later. + */ + g_mutex_lock (&dconf_engine_global_lock); + if (g_atomic_int_get (&engine->ref_count) != 0) + { + g_mutex_unlock (&dconf_engine_global_lock); + return; + } - for (i = 0; i < engine->n_sources; i++) - dconf_engine_source_free (engine->sources[i]); + /* It's possible that another thread grabbed a reference at the + * last minute and dropped it back to zero again (thus causing the + * above check to pass). In that case, however, the other thread + * will have also dropped the refcount from 1 to 0 and be inside + * of the dec-and-test above. + * + * We can only have one of the two threads doing the freeing of + * the data, so we have a simple rule: the thread that removes the + * engine from the global list is the one that does the free. + * Since this operation is performed under mutex we can be sure + * that only one thread will win. + */ + if (!g_slist_find (dconf_engine_global_list, engine)) + { + g_mutex_unlock (&dconf_engine_global_lock); + return; + } + + dconf_engine_global_list = g_slist_remove (dconf_engine_global_list, engine); + g_mutex_unlock (&dconf_engine_global_lock); + + g_mutex_clear (&engine->sources_lock); + g_mutex_clear (&engine->queue_lock); + + g_free (engine->last_handled); + + for (i = 0; i < engine->n_sources; i++) + dconf_engine_source_free (engine->sources[i]); - g_free (engine->sources); + g_free (engine->sources); + + if (engine->free_func) + engine->free_func (engine->user_data); + + g_slice_free (DConfEngine, engine); + } +} + +static DConfEngine * +dconf_engine_ref (DConfEngine *engine) +{ + g_atomic_int_inc (&engine->ref_count); - g_slice_free (DConfEngine, engine); + return engine; } guint64 @@ -586,7 +655,7 @@ dconf_engine_call_handle_new (DConfEngine *engine, g_assert (size >= sizeof (DConfEngineCallHandle)); handle = g_malloc0 (size); - handle->engine = engine; + handle->engine = dconf_engine_ref (engine); handle->callback = callback; handle->expected_reply = expected_reply; @@ -607,6 +676,7 @@ dconf_engine_call_handle_reply (DConfEngineCallHandle *handle, static void dconf_engine_call_handle_free (DConfEngineCallHandle *handle) { + dconf_engine_unref (handle->engine); g_free (handle); } @@ -825,16 +895,28 @@ dconf_engine_change_completed (DConfEngine *engine, * making room for another to be added. Check that. */ dconf_engine_manage_queue (engine); + dconf_engine_unlock_queues (engine); /* Deal with the reply we got. */ if (reply) { + /* The write worked. + * + * We already sent a change notification for this item when we + * added it to the pending queue and we don't want to send another + * one again. At the same time, it's very likely that we're just + * about to receive a change signal from the service. + * + * The tag sent as part of the reply to the Change call will be + * the same tag as on the change notification signal. Record that + * tag so that we can ignore the signal when it comes. + * + * last_handled is only ever touched from the worker thread + */ g_free (engine->last_handled); g_variant_get (reply, "(s)", &engine->last_handled); } - dconf_engine_unlock_queues (engine); - if (error) { /* Some kind of unexpected failure occured while attempting to @@ -1005,5 +1087,48 @@ dconf_engine_handle_dbus_signal (GBusType type, const gchar *member, GVariant *body) { - g_print ("saw a sig"); + if (g_str_equal (member, "Notify")) + { + const gchar *prefix; + const gchar **changes; + const gchar *tag; + GSList *engines; + + if (!g_variant_is_of_type (body, G_VARIANT_TYPE ("(sass)"))) + return; + + g_variant_get (body, "(&s^a&s&s)", &prefix, &changes, &tag); + + g_mutex_lock (&dconf_engine_global_lock); + engines = g_slist_copy_deep (dconf_engine_global_list, (GCopyFunc) dconf_engine_ref, NULL); + g_mutex_unlock (&dconf_engine_global_lock); + + while (engines) + { + DConfEngine *engine = engines->data; + + /* It's possible that this incoming change notify is for a + * change that we already announced to the client when we + * placed it in the pending queue. + * + * Check last_handled to determine if we should ignore it. + */ + if (!engine->last_handled || !g_str_equal (engine->last_handled, tag)) + dconf_engine_change_notify (engine, prefix, changes, tag, engine->user_data); + + engines = g_slist_delete_link (engines, engines); + + dconf_engine_unref (engine); + } + + g_free (changes); + } + + else if (g_str_equal (member, "WritabilityNotify")) + { + if (!g_variant_is_of_type (body, G_VARIANT_TYPE ("(s)"))) + return; + + g_warning ("Need to handle writability changes"); /* XXX */ + } } diff --git a/engine/dconf-engine.h b/engine/dconf-engine.h index 19b7097..bad8cf1 100644 --- a/engine/dconf-engine.h +++ b/engine/dconf-engine.h @@ -100,10 +100,11 @@ void dconf_engine_handle_dbus_signal (GBusTyp GVariant *parameters); G_GNUC_INTERNAL -DConfEngine * dconf_engine_new (gpointer user_data); +DConfEngine * dconf_engine_new (gpointer user_data, + GDestroyNotify free_func); G_GNUC_INTERNAL -void dconf_engine_free (DConfEngine *engine); +void dconf_engine_unref (DConfEngine *engine); /* Read API: always handled immediately */ G_GNUC_INTERNAL diff --git a/gsettings/dconfsettingsbackend.c b/gsettings/dconfsettingsbackend.c index 85aef4e..53a982c 100644 --- a/gsettings/dconfsettingsbackend.c +++ b/gsettings/dconfsettingsbackend.c @@ -141,9 +141,22 @@ dconf_settings_backend_sync (GSettingsBackend *backend) } static void +dconf_settings_backend_free_weak_ref (gpointer data) +{ + GWeakRef *weak_ref = data; + + g_weak_ref_clear (weak_ref); + g_slice_free (GWeakRef, weak_ref); +} + +static void dconf_settings_backend_init (DConfSettingsBackend *dcsb) { - dcsb->engine = dconf_engine_new (dcsb); + GWeakRef *weak_ref; + + weak_ref = g_slice_new (GWeakRef); + g_weak_ref_init (weak_ref, dcsb); + dcsb->engine = dconf_engine_new (weak_ref, dconf_settings_backend_free_weak_ref); } static void @@ -151,7 +164,7 @@ dconf_settings_backend_finalize (GObject *object) { DConfSettingsBackend *dcsb = (DConfSettingsBackend *) object; - dconf_engine_free (dcsb->engine); + dconf_engine_unref (dcsb->engine); G_OBJECT_CLASS (dconf_settings_backend_parent_class) ->finalize (object); @@ -202,4 +215,24 @@ dconf_engine_change_notify (DConfEngine *engine, const gchar *tag, gpointer user_data) { + GWeakRef *weak_ref = user_data; + DConfSettingsBackend *dcsb; + + dcsb = g_weak_ref_get (weak_ref); + + if (dcsb == NULL) + return; + + if (changes[0] == NULL) + return; + + if (changes[1] == NULL) + { + if (g_str_has_suffix (prefix, "/")) + g_settings_backend_path_changed (G_SETTINGS_BACKEND (dcsb), prefix, NULL); + else + g_settings_backend_changed (G_SETTINGS_BACKEND (dcsb), prefix, NULL); + } + else + g_settings_backend_keys_changed (G_SETTINGS_BACKEND (dcsb), prefix, changes, NULL); } |