rpms
/
dconf
7
0
Fork 0
Code Issues Pull Requests Releases Activity
dconf/SOURCES/0002-Engine-extend-subscrip...

406 lines
16 KiB
Diff
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

From 0986a258cc1df8c1e2aa17a0c2138d178405f902 Mon Sep 17 00:00:00 2001
From: Daniel Playfair Cal <daniel.playfair.cal@gmail.com>
Date: Wed, 25 Jul 2018 00:52:24 +1000
Subject: [PATCH 2/5] Engine: extend subscriptions state to account for
multiple client subscriptions to the same path
Remove accidental whitespace change
Simplify branching in watch_fast and unwatch_fast
Indentation fixes
Store the subscription counts directly in the hash table pointer instead of mallocing ints
Add documentation comments for new utility functions
---
engine/dconf-engine.c | 191 ++++++++++++++++++++++++++++--------------
engine/dconf-engine.h | 11 ---
tests/engine.c | 54 +-----------
3 files changed, 133 insertions(+), 123 deletions(-)
diff --git a/engine/dconf-engine.c b/engine/dconf-engine.c
index 2a99eab..1963c34 100644
--- a/engine/dconf-engine.c
+++ b/engine/dconf-engine.c
@@ -170,8 +170,14 @@ struct _DConfEngine
gchar *last_handled; /* reply tag from last item in in_flight */
- GHashTable *watched_paths; /* list of paths currently being watched for changes */
- GHashTable *pending_paths; /* list of paths waiting to enter watched state */
+ /**
+ * establishing and active, are hash tables storing the number
+ * of subscriptions to each path in the two possible states
+ */
+ /* 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 */
+ GHashTable *active;
};
/* When taking the sources lock we check if any of the databases have
@@ -225,6 +231,78 @@ dconf_engine_unlock_queues (DConfEngine *engine)
g_mutex_unlock (&engine->queue_lock);
}
+/**
+ * Adds the count of subscriptions to @path in @from_table to the
+ * corresponding count in @to_table, creating it if it did not exist.
+ * Removes the count from @from_table.
+ */
+static void
+dconf_engine_move_subscriptions (GHashTable *from_counts,
+ GHashTable *to_counts,
+ const gchar *path)
+{
+ guint from_count = GPOINTER_TO_UINT (g_hash_table_lookup (from_counts, path));
+ guint old_to_count = GPOINTER_TO_UINT (g_hash_table_lookup (to_counts, path));
+ // Detect overflows
+ g_assert (old_to_count <= G_MAXUINT32 - from_count);
+ guint new_to_count = old_to_count + from_count;
+ if (from_count != 0)
+ {
+ g_hash_table_remove (from_counts, path);
+ g_hash_table_replace (to_counts,
+ g_strdup (path),
+ GUINT_TO_POINTER (new_to_count));
+ }
+}
+
+/**
+ * Increments the reference count for the subscription to @path, or sets
+ * it to 1 if it didn’t previously exist.
+ * Returns the new reference count.
+ */
+static guint
+dconf_engine_inc_subscriptions (GHashTable *counts,
+ const gchar *path)
+{
+ guint old_count = GPOINTER_TO_UINT (g_hash_table_lookup (counts, path));
+ // Detect overflows
+ g_assert (old_count < G_MAXUINT32);
+ guint new_count = old_count + 1;
+ g_hash_table_replace (counts, g_strdup (path), GUINT_TO_POINTER (new_count));
+ return new_count;
+}
+
+/**
+ * Decrements the reference count for the subscription to @path, or
+ * removes it if the new value is 0. The count must exist and be greater
+ * than 0.
+ * Returns the new reference count, or 0 if it does not exist.
+ */
+static guint
+dconf_engine_dec_subscriptions (GHashTable *counts,
+ const gchar *path)
+{
+ guint old_count = GPOINTER_TO_UINT (g_hash_table_lookup (counts, path));
+ g_assert (old_count > 0);
+ guint new_count = old_count - 1;
+ if (new_count == 0)
+ g_hash_table_remove (counts, path);
+ else
+ g_hash_table_replace (counts, g_strdup (path), GUINT_TO_POINTER (new_count));
+ return new_count;
+}
+
+/**
+ * Returns the reference count for the subscription to @path, or 0 if it
+ * does not exist.
+ */
+static guint
+dconf_engine_count_subscriptions (GHashTable *counts,
+ const gchar *path)
+{
+ return GPOINTER_TO_UINT (g_hash_table_lookup (counts, path));
+}
+
DConfEngine *
dconf_engine_new (const gchar *profile,
gpointer user_data,
@@ -247,8 +325,14 @@ 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);
- engine->watched_paths = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
- engine->pending_paths = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
+ engine->establishing = g_hash_table_new_full (g_str_hash,
+ g_str_equal,
+ g_free,
+ NULL);
+ engine->active = g_hash_table_new_full (g_str_hash,
+ g_str_equal,
+ g_free,
+ NULL);
return engine;
}
@@ -300,8 +384,8 @@ dconf_engine_unref (DConfEngine *engine)
g_free (engine->sources);
- g_hash_table_unref(engine->pending_paths);
- g_hash_table_unref(engine->watched_paths);
+ g_hash_table_unref (engine->establishing);
+ g_hash_table_unref (engine->active);
if (engine->free_func)
engine->free_func (engine->user_data);
@@ -847,7 +931,14 @@ dconf_engine_watch_established (DConfEngine *engine,
dconf_engine_change_notify (engine, ow->path, changes, NULL, FALSE, NULL, engine->user_data);
}
- dconf_engine_set_watching (engine, ow->path, TRUE, TRUE);
+ guint num_establishing = dconf_engine_count_subscriptions (engine->establishing,
+ ow->path);
+ if (num_establishing > 0)
+ // Subscription(s): establishing -> active
+ dconf_engine_move_subscriptions (engine->establishing,
+ engine->active,
+ ow->path);
+
dconf_engine_call_handle_free (handle);
}
@@ -855,14 +946,17 @@ void
dconf_engine_watch_fast (DConfEngine *engine,
const gchar *path)
{
- if (dconf_engine_is_watching (engine, path, TRUE))
- {
- /**
- * Either there is already a match rule in place for this exact path,
- * or there is already a request in progress to add a match.
- */
- return;
- }
+ guint num_establishing = dconf_engine_count_subscriptions (engine->establishing, path);
+ guint num_active = dconf_engine_count_subscriptions (engine->active, path);
+ if (num_active > 0)
+ // Subscription: inactive -> active
+ dconf_engine_inc_subscriptions (engine->active, path);
+ else
+ // Subscription: inactive -> establishing
+ num_establishing = dconf_engine_inc_subscriptions (engine->establishing,
+ path);
+ if (num_establishing > 1 || num_active > 0)
+ return;
OutstandingWatch *ow;
gint i;
@@ -897,23 +991,33 @@ dconf_engine_watch_fast (DConfEngine *engine,
"/org/freedesktop/DBus", "org.freedesktop.DBus", "AddMatch",
dconf_engine_make_match_rule (engine->sources[i], path),
&ow->handle, NULL);
-
- dconf_engine_set_watching (engine, ow->path, TRUE, FALSE);
}
void
dconf_engine_unwatch_fast (DConfEngine *engine,
const gchar *path)
{
+ guint num_active = dconf_engine_count_subscriptions (engine->active, path);
+ guint num_establishing = dconf_engine_count_subscriptions (engine->establishing, path);
gint i;
+ // Client code cannot unsubscribe if it is not subscribed
+ g_assert (num_active > 0 || num_establishing > 0);
+ if (num_active == 0)
+ // Subscription: establishing -> inactive
+ num_establishing = dconf_engine_dec_subscriptions (engine->establishing, path);
+ else
+ // Subscription: active -> inactive
+ num_active = dconf_engine_dec_subscriptions (engine->active, path);
+
+ if (num_active > 0 || num_establishing > 0)
+ return;
+
for (i = 0; i < engine->n_sources; i++)
if (engine->sources[i]->bus_type)
dconf_engine_dbus_call_async_func (engine->sources[i]->bus_type, "org.freedesktop.DBus",
"/org/freedesktop/DBus", "org.freedesktop.DBus", "RemoveMatch",
dconf_engine_make_match_rule (engine->sources[i], path), NULL, NULL);
-
- dconf_engine_set_watching (engine, path, FALSE, FALSE);
}
static void
@@ -951,16 +1055,18 @@ void
dconf_engine_watch_sync (DConfEngine *engine,
const gchar *path)
{
- dconf_engine_handle_match_rule_sync (engine, "AddMatch", path);
- dconf_engine_set_watching (engine, path, TRUE, TRUE);
+ guint num_active = dconf_engine_inc_subscriptions (engine->active, path);
+ if (num_active == 1)
+ dconf_engine_handle_match_rule_sync (engine, "AddMatch", path);
}
void
dconf_engine_unwatch_sync (DConfEngine *engine,
const gchar *path)
{
- dconf_engine_handle_match_rule_sync (engine, "RemoveMatch", path);
- dconf_engine_set_watching (engine, path, FALSE, FALSE);
+ guint num_active = dconf_engine_dec_subscriptions (engine->active, path);
+ if (num_active == 0)
+ dconf_engine_handle_match_rule_sync (engine, "RemoveMatch", path);
}
typedef struct
@@ -1418,42 +1524,3 @@ dconf_engine_sync (DConfEngine *engine)
g_cond_wait (&engine->queue_cond, &engine->queue_lock);
dconf_engine_unlock_queues (engine);
}
-
-void
-dconf_engine_set_watching (DConfEngine *engine,
- const gchar *path,
- const gboolean is_watching,
- const gboolean is_established)
-{
- if (is_watching)
- {
- if (is_established)
- {
- g_hash_table_add (engine->watched_paths, g_strdup (path));
- g_hash_table_remove (engine->pending_paths, path);
- }
- else
- {
- g_hash_table_add (engine->pending_paths, g_strdup (path));
- g_hash_table_remove (engine->watched_paths, path);
- }
- }
- else
- {
- g_hash_table_remove (engine->watched_paths, path);
- g_hash_table_remove (engine->pending_paths, path);
- }
-}
-
-gboolean
-dconf_engine_is_watching (DConfEngine *engine, const gchar *path, const gboolean only_established)
-{
- gconstpointer key = (gconstpointer) path;
- if (g_hash_table_contains (engine->watched_paths, key))
- return TRUE;
-
- if (!only_established && g_hash_table_contains (engine->pending_paths, key))
- return TRUE;
-
- return FALSE;
-}
diff --git a/engine/dconf-engine.h b/engine/dconf-engine.h
index 06ed5a7..2485423 100644
--- a/engine/dconf-engine.h
+++ b/engine/dconf-engine.h
@@ -104,17 +104,6 @@ DConfEngine * dconf_engine_new (const g
G_GNUC_INTERNAL
void dconf_engine_unref (DConfEngine *engine);
-G_GNUC_INTERNAL
-void dconf_engine_set_watching (DConfEngine *engine,
- const gchar *path,
- const gboolean is_watching,
- const gboolean is_established);
-
-G_GNUC_INTERNAL
-gboolean dconf_engine_is_watching (DConfEngine *engine,
- const gchar *path,
- const gboolean only_established);
-
/* Read API: always handled immediately */
G_GNUC_INTERNAL
guint64 dconf_engine_get_state (DConfEngine *engine);
diff --git a/tests/engine.c b/tests/engine.c
index aa1db1c..038c04c 100644
--- a/tests/engine.c
+++ b/tests/engine.c
@@ -1210,13 +1210,16 @@ test_watch_fast (void)
c = dconf_engine_get_state (engine);
g_assert_cmpuint (b, ==, c);
/* The watch result was not sent, because the path was already watched */
- dconf_mock_dbus_assert_no_async();
+ dconf_mock_dbus_assert_no_async ();
c = dconf_engine_get_state (engine);
g_assert_cmpuint (b, ==, c);
/* Since the path was already being watched,
* do not expect a second false change notification */
g_assert_cmpstr (change_log->str, ==, "/a/b/c:1::nil;");
dconf_engine_unwatch_fast (engine, "/a/b/c");
+ /* nothing was done, because there is still a subscription left */
+ dconf_mock_dbus_assert_no_async ();
+ dconf_engine_unwatch_fast (engine, "/a/b/c");
dconf_mock_dbus_async_reply (triv, NULL);
dconf_mock_dbus_async_reply (triv, NULL);
dconf_mock_dbus_assert_no_async ();
@@ -1286,54 +1289,6 @@ test_watch_sync (void)
match_request_type = NULL;
}
-static void
-test_watching (void)
-{
- DConfEngine *engine;
- const gchar *apple = "apple";
- const gchar *orange = "orange";
- const gchar *banana = "banana";
-
- engine = dconf_engine_new (SRCDIR "/profile/dos", NULL, NULL);
-
- g_assert (!dconf_engine_is_watching(engine, apple, TRUE));
- g_assert (!dconf_engine_is_watching(engine, apple, FALSE));
- g_assert (!dconf_engine_is_watching(engine, orange, TRUE));
- g_assert (!dconf_engine_is_watching(engine, orange, FALSE));
- g_assert (!dconf_engine_is_watching(engine, banana, TRUE));
- g_assert (!dconf_engine_is_watching(engine, banana, FALSE));
-
- dconf_engine_set_watching (engine, apple, FALSE, FALSE);
- dconf_engine_set_watching (engine, orange, TRUE, FALSE);
- dconf_engine_set_watching (engine, banana, TRUE, TRUE);
-
- g_assert (!dconf_engine_is_watching(engine, apple, TRUE));
- g_assert (!dconf_engine_is_watching(engine, apple, FALSE));
- g_assert (!dconf_engine_is_watching(engine, orange, TRUE));
- g_assert (dconf_engine_is_watching(engine, orange, FALSE));
- g_assert (dconf_engine_is_watching(engine, banana, TRUE));
- g_assert (dconf_engine_is_watching(engine, banana, FALSE));
-
- dconf_engine_set_watching (engine, orange, TRUE, TRUE);
- dconf_engine_set_watching (engine, banana, FALSE, FALSE);
-
- g_assert (!dconf_engine_is_watching(engine, apple, TRUE));
- g_assert (!dconf_engine_is_watching(engine, apple, FALSE));
- g_assert (dconf_engine_is_watching(engine, orange, TRUE));
- g_assert (dconf_engine_is_watching(engine, orange, FALSE));
- g_assert (!dconf_engine_is_watching(engine, banana, TRUE));
- g_assert (!dconf_engine_is_watching(engine, banana, FALSE));
-
- dconf_engine_set_watching (engine, orange, FALSE, FALSE);
-
- g_assert (!dconf_engine_is_watching(engine, apple, TRUE));
- g_assert (!dconf_engine_is_watching(engine, apple, FALSE));
- g_assert (!dconf_engine_is_watching(engine, orange, TRUE));
- g_assert (!dconf_engine_is_watching(engine, orange, FALSE));
- g_assert (!dconf_engine_is_watching(engine, banana, TRUE));
- g_assert (!dconf_engine_is_watching(engine, banana, FALSE));
-}
-
static void
test_change_fast (void)
{
@@ -1819,7 +1774,6 @@ main (int argc, char **argv)
g_test_add_func ("/engine/read", test_read);
g_test_add_func ("/engine/watch/fast", test_watch_fast);
g_test_add_func ("/engine/watch/sync", test_watch_sync);
- g_test_add_func ("/engine/watch/watching", test_watching);
g_test_add_func ("/engine/change/fast", test_change_fast);
g_test_add_func ("/engine/change/sync", test_change_sync);
g_test_add_func ("/engine/signals", test_signals);
--
2.20.1
Reference in New Issue View Git Blame Copy Permalink