rpms/dconf
7
0
Fork 0
Code Issues Pull Requests Releases Activity
1a809529b4
dconf/0001-Engine-track-in-progress-watch-handles-to-avoid-spur.patch

308 lines
12 KiB
Diff
Raw Blame History

From d148fffb58935d69a20dcc42b3ce3d998742f0e7 Mon Sep 17 00:00:00 2001
From: Daniel Playfair Cal <daniel.playfair.cal@gmail.com>
Date: Fri, 13 Jul 2018 14:47:45 +0000
Subject: [PATCH] Engine: track in progress watch handles to avoid spurious
changed signals for the root path
---
engine/dconf-engine.c | 88 +++++++++++++++++++++++++++++++++++++------
engine/dconf-engine.h | 11 ++++++
tests/engine.c | 66 +++++++++++++++++++++++++++++++-
3 files changed, 151 insertions(+), 14 deletions(-)
diff --git a/engine/dconf-engine.c b/engine/dconf-engine.c
index bc36e52..c0ff12d 100644
--- a/engine/dconf-engine.c
+++ b/engine/dconf-engine.c
@@ -158,17 +158,20 @@ struct _DConfEngine
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. */
+ 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, queue_cond */
- GCond queue_cond; /* Signalled when the queues empty */
- GQueue pending; /* DConfChangeset */
- GQueue in_flight; /* DConfChangeset */
+ GMutex queue_lock; /* This lock is for pending, in_flight, queue_cond */
+ GCond queue_cond; /* Signalled when the queues empty */
+ GQueue pending; /* DConfChangeset */
+ GQueue in_flight; /* DConfChangeset */
- gchar *last_handled; /* reply tag from last item in in_flight */
+ 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 */
};
/* When taking the sources lock we check if any of the databases have
@@ -244,6 +247,9 @@ 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);
+
return engine;
}
@@ -799,8 +805,9 @@ typedef struct
{
DConfEngineCallHandle handle;
- guint64 state;
- gint pending;
+ guint64 state;
+ gint pending;
+ gchar *path;
} OutstandingWatch;
static void
@@ -825,11 +832,13 @@ dconf_engine_watch_established (DConfEngine *engine,
* must have changed while our watch requests were on the wire.
*
* We don't know what changed, so we can just say that potentially
- * everything changed. This case is very rare, anyway...
+ * everything under the path being watched changed. This case is
+ * very rare, anyway...
*/
- dconf_engine_change_notify (engine, "/", changes, NULL, FALSE, NULL, engine->user_data);
+ dconf_engine_change_notify (engine, ow->path, changes, NULL, FALSE, NULL, engine->user_data);
}
+ dconf_engine_set_watching (engine, ow->path, TRUE, TRUE);
dconf_engine_call_handle_free (handle);
}
@@ -837,6 +846,15 @@ 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;
+ }
+
OutstandingWatch *ow;
gint i;
@@ -855,6 +873,7 @@ dconf_engine_watch_fast (DConfEngine *engine,
ow = dconf_engine_call_handle_new (engine, dconf_engine_watch_established,
G_VARIANT_TYPE_UNIT, sizeof (OutstandingWatch));
ow->state = dconf_engine_get_state (engine);
+ ow->path = g_strdup (path);
/* We start getting async calls returned as soon as we start dispatching them,
* so we must not touch the 'ow' struct after we send the first one.
@@ -869,6 +888,8 @@ 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
@@ -882,6 +903,8 @@ dconf_engine_unwatch_fast (DConfEngine *engine,
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
@@ -920,6 +943,7 @@ 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);
}
void
@@ -927,6 +951,7 @@ 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);
}
typedef struct
@@ -1384,3 +1409,42 @@ 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 2485423..06ed5a7 100644
--- a/engine/dconf-engine.h
+++ b/engine/dconf-engine.h
@@ -104,6 +104,17 @@ 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 a804b9a..aa1db1c 100644
--- a/tests/engine.c
+++ b/tests/engine.c
@@ -1153,7 +1153,7 @@ test_watch_fast (void)
DConfEngine *engine;
GvdbTable *table;
GVariant *triv;
- guint64 a, b;
+ guint64 a, b, c;
change_log = g_string_new (NULL);
@@ -1202,7 +1202,20 @@ test_watch_fast (void)
dconf_mock_dbus_assert_no_async ();
b = dconf_engine_get_state (engine);
g_assert_cmpuint (a, !=, b);
- g_assert_cmpstr (change_log->str, ==, "/:1::nil;");
+ g_assert_cmpstr (change_log->str, ==, "/a/b/c:1::nil;");
+ /* Try to establish a watch again for the same path */
+ dconf_engine_watch_fast (engine, "/a/b/c");
+ g_assert (!dconf_engine_has_outstanding (engine));
+ dconf_engine_sync (engine);
+ 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();
+ 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");
dconf_mock_dbus_async_reply (triv, NULL);
dconf_mock_dbus_async_reply (triv, NULL);
@@ -1273,6 +1286,54 @@ 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)
{
@@ -1758,6 +1819,7 @@ 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