168 lines
6.7 KiB
Diff
168 lines
6.7 KiB
Diff
From 21711aa40bed4e61bba7d5f9fee141825fe76823 Mon Sep 17 00:00:00 2001
|
|
From: Daniel Playfair Cal <daniel.playfair.cal@gmail.com>
|
|
Date: Thu, 26 Jul 2018 00:00:09 +1000
|
|
Subject: [PATCH 4/5] Engine: Add locks around access to subscription counts to
|
|
ensure that each state transition is atomic
|
|
|
|
Update comment about threading, documenting the new lock
|
|
|
|
Add documentation comments for new utility functions
|
|
---
|
|
engine/dconf-engine.c | 47 ++++++++++++++++++++++++++++++++++++++++---
|
|
1 file changed, 44 insertions(+), 3 deletions(-)
|
|
|
|
diff --git a/engine/dconf-engine.c b/engine/dconf-engine.c
|
|
index 2911724..ad891e6 100644
|
|
--- a/engine/dconf-engine.c
|
|
+++ b/engine/dconf-engine.c
|
|
@@ -128,7 +128,7 @@
|
|
* it is willing to deal with receiving the change notifies in those
|
|
* threads.
|
|
*
|
|
- * Thread-safety is implemented using two locks.
|
|
+ * Thread-safety is implemented using three locks.
|
|
*
|
|
* The first lock (sources_lock) protects the sources. Although the
|
|
* sources are only ever read from, it is necessary to lock them because
|
|
@@ -143,8 +143,15 @@
|
|
* The second lock (queue_lock) protects the various queues that are
|
|
* used to implement the "fast" writes described above.
|
|
*
|
|
- * If both locks are held at the same time then the sources lock must
|
|
- * have been acquired first.
|
|
+ * The third lock (subscription_count_lock) protects the two hash tables
|
|
+ * that are used to keep track of the number of subscriptions held by
|
|
+ * the client library to each path.
|
|
+ *
|
|
+ * If sources_lock and queue_lock are held at the same time then then
|
|
+ * sources_lock must have been acquired first.
|
|
+ *
|
|
+ * subscription_count_lock is never held at the same time as
|
|
+ * sources_lock or queue_lock
|
|
*/
|
|
|
|
#define MAX_IN_FLIGHT 2
|
|
@@ -174,6 +181,8 @@ struct _DConfEngine
|
|
* establishing and active, are hash tables storing the number
|
|
* of subscriptions to each path in the two possible states
|
|
*/
|
|
+ /* This lock ensures that transactions involving subscription counts are atomic */
|
|
+ GMutex subscription_count_lock;
|
|
/* active on the client side, but awaiting confirmation from the writer */
|
|
GHashTable *establishing;
|
|
/* active on the client side, and with a D-Bus match rule established */
|
|
@@ -303,6 +312,25 @@ dconf_engine_count_subscriptions (GHashTable *counts,
|
|
return GPOINTER_TO_UINT (g_hash_table_lookup (counts, path));
|
|
}
|
|
|
|
+/**
|
|
+ * Acquires the subscription counts lock, which must be held when
|
|
+ * reading or writing to the subscription counts.
|
|
+ */
|
|
+static void
|
|
+dconf_engine_lock_subscription_counts (DConfEngine *engine)
|
|
+{
|
|
+ g_mutex_lock (&engine->subscription_count_lock);
|
|
+}
|
|
+
|
|
+/**
|
|
+ * Releases the subscription counts lock
|
|
+ */
|
|
+static void
|
|
+dconf_engine_unlock_subscription_counts (DConfEngine *engine)
|
|
+{
|
|
+ g_mutex_unlock (&engine->subscription_count_lock);
|
|
+}
|
|
+
|
|
DConfEngine *
|
|
dconf_engine_new (const gchar *profile,
|
|
gpointer user_data,
|
|
@@ -325,6 +353,7 @@ dconf_engine_new (const gchar *profile,
|
|
dconf_engine_global_list = g_slist_prepend (dconf_engine_global_list, engine);
|
|
g_mutex_unlock (&dconf_engine_global_lock);
|
|
|
|
+ g_mutex_init (&engine->subscription_count_lock);
|
|
engine->establishing = g_hash_table_new_full (g_str_hash,
|
|
g_str_equal,
|
|
g_free,
|
|
@@ -387,6 +416,8 @@ dconf_engine_unref (DConfEngine *engine)
|
|
g_hash_table_unref (engine->establishing);
|
|
g_hash_table_unref (engine->active);
|
|
|
|
+ g_mutex_clear (&engine->subscription_count_lock);
|
|
+
|
|
if (engine->free_func)
|
|
engine->free_func (engine->user_data);
|
|
|
|
@@ -932,6 +963,7 @@ dconf_engine_watch_established (DConfEngine *engine,
|
|
dconf_engine_change_notify (engine, ow->path, changes, NULL, FALSE, NULL, engine->user_data);
|
|
}
|
|
|
|
+ dconf_engine_lock_subscription_counts (engine);
|
|
guint num_establishing = dconf_engine_count_subscriptions (engine->establishing,
|
|
ow->path);
|
|
g_debug ("watch_established: \"%s\" (establishing: %d)", ow->path, num_establishing);
|
|
@@ -941,6 +973,7 @@ dconf_engine_watch_established (DConfEngine *engine,
|
|
engine->active,
|
|
ow->path);
|
|
|
|
+ dconf_engine_unlock_subscription_counts (engine);
|
|
dconf_engine_call_handle_free (handle);
|
|
}
|
|
|
|
@@ -948,6 +981,7 @@ void
|
|
dconf_engine_watch_fast (DConfEngine *engine,
|
|
const gchar *path)
|
|
{
|
|
+ dconf_engine_lock_subscription_counts (engine);
|
|
guint num_establishing = dconf_engine_count_subscriptions (engine->establishing, path);
|
|
guint num_active = dconf_engine_count_subscriptions (engine->active, path);
|
|
g_debug ("watch_fast: \"%s\" (establishing: %d, active: %d)", path, num_establishing, num_active);
|
|
@@ -958,6 +992,7 @@ dconf_engine_watch_fast (DConfEngine *engine,
|
|
// Subscription: inactive -> establishing
|
|
num_establishing = dconf_engine_inc_subscriptions (engine->establishing,
|
|
path);
|
|
+ dconf_engine_unlock_subscription_counts (engine);
|
|
if (num_establishing > 1 || num_active > 0)
|
|
return;
|
|
|
|
@@ -1000,6 +1035,7 @@ void
|
|
dconf_engine_unwatch_fast (DConfEngine *engine,
|
|
const gchar *path)
|
|
{
|
|
+ dconf_engine_lock_subscription_counts (engine);
|
|
guint num_active = dconf_engine_count_subscriptions (engine->active, path);
|
|
guint num_establishing = dconf_engine_count_subscriptions (engine->establishing, path);
|
|
gint i;
|
|
@@ -1014,6 +1050,7 @@ dconf_engine_unwatch_fast (DConfEngine *engine,
|
|
// Subscription: active -> inactive
|
|
num_active = dconf_engine_dec_subscriptions (engine->active, path);
|
|
|
|
+ dconf_engine_unlock_subscription_counts (engine);
|
|
if (num_active > 0 || num_establishing > 0)
|
|
return;
|
|
|
|
@@ -1059,7 +1096,9 @@ void
|
|
dconf_engine_watch_sync (DConfEngine *engine,
|
|
const gchar *path)
|
|
{
|
|
+ dconf_engine_lock_subscription_counts (engine);
|
|
guint num_active = dconf_engine_inc_subscriptions (engine->active, path);
|
|
+ dconf_engine_unlock_subscription_counts (engine);
|
|
g_debug ("watch_sync: \"%s\" (active: %d)", path, num_active - 1);
|
|
if (num_active == 1)
|
|
dconf_engine_handle_match_rule_sync (engine, "AddMatch", path);
|
|
@@ -1069,7 +1108,9 @@ void
|
|
dconf_engine_unwatch_sync (DConfEngine *engine,
|
|
const gchar *path)
|
|
{
|
|
+ dconf_engine_lock_subscription_counts (engine);
|
|
guint num_active = dconf_engine_dec_subscriptions (engine->active, path);
|
|
+ dconf_engine_unlock_subscription_counts (engine);
|
|
g_debug ("unwatch_sync: \"%s\" (active: %d)", path, num_active + 1);
|
|
if (num_active == 0)
|
|
dconf_engine_handle_match_rule_sync (engine, "RemoveMatch", path);
|
|
--
|
|
2.20.1
|
|
|