Compare commits

..

No commits in common. "c9-beta" and "c8" have entirely different histories.
c9-beta ... c8

13 changed files with 1413 additions and 133 deletions

View File

@ -1 +1 @@
c8e12b98b2b10ccae4ee13395a39b3e913f58ab6 SOURCES/dconf-0.40.0.tar.xz e6b10da51df002a1661ce938941770c549cd5b87 SOURCES/dconf-0.28.0.tar.xz

2
.gitignore vendored
View File

@ -1 +1 @@
SOURCES/dconf-0.40.0.tar.xz SOURCES/dconf-0.28.0.tar.xz

View File

@ -0,0 +1,35 @@
From 63c985f5afecc69eee3fe7a7e04cc8118097b3c3 Mon Sep 17 00:00:00 2001
From: Daniel Playfair Cal <daniel.playfair.cal@gmail.com>
Date: Wed, 15 Aug 2018 08:46:49 +1000
Subject: [PATCH] Engine: Change overflow thresholds in subscription counts
from GMAXUINT32 to GMAXUINT
---
engine/dconf-engine.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/engine/dconf-engine.c b/engine/dconf-engine.c
index ad891e6..dde8c18 100644
--- a/engine/dconf-engine.c
+++ b/engine/dconf-engine.c
@@ -253,7 +253,7 @@ dconf_engine_move_subscriptions (GHashTable *from_counts,
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);
+ g_assert (old_to_count <= G_MAXUINT - from_count);
guint new_to_count = old_to_count + from_count;
if (from_count != 0)
{
@@ -275,7 +275,7 @@ dconf_engine_inc_subscriptions (GHashTable *counts,
{
guint old_count = GPOINTER_TO_UINT (g_hash_table_lookup (counts, path));
// Detect overflows
- g_assert (old_count < G_MAXUINT32);
+ g_assert (old_count < G_MAXUINT);
guint new_count = old_count + 1;
g_hash_table_replace (counts, g_strdup (path), GUINT_TO_POINTER (new_count));
return new_count;
--
2.20.1

View File

@ -0,0 +1,37 @@
From 6e6089791b38b4da84d5b49c0b24286be1526a2b Mon Sep 17 00:00:00 2001
From: Daniel Playfair Cal <daniel.playfair.cal@gmail.com>
Date: Wed, 25 Jul 2018 23:58:26 +1000
Subject: [PATCH 1/5] Engine: add some missing objects to dconf_engine_unref
---
engine/dconf-engine.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/engine/dconf-engine.c b/engine/dconf-engine.c
index c0ff12d..2a99eab 100644
--- a/engine/dconf-engine.c
+++ b/engine/dconf-engine.c
@@ -289,11 +289,20 @@ dconf_engine_unref (DConfEngine *engine)
g_free (engine->last_handled);
+ while (!g_queue_is_empty (&engine->pending))
+ dconf_changeset_unref ((DConfChangeset *) g_queue_pop_head (&engine->pending));
+
+ while (!g_queue_is_empty (&engine->in_flight))
+ dconf_changeset_unref ((DConfChangeset *) g_queue_pop_head (&engine->in_flight));
+
for (i = 0; i < engine->n_sources; i++)
dconf_engine_source_free (engine->sources[i]);
g_free (engine->sources);
+ g_hash_table_unref(engine->pending_paths);
+ g_hash_table_unref(engine->watched_paths);
+
if (engine->free_func)
engine->free_func (engine->user_data);
--
2.20.1

View File

@ -0,0 +1,307 @@
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

View File

@ -0,0 +1,405 @@
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 didnt 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

View File

@ -0,0 +1,101 @@
From d970e6a07e82449c7d93b0314403af321230e081 Mon Sep 17 00:00:00 2001
From: Daniel Playfair Cal <daniel.playfair.cal@gmail.com>
Date: Wed, 25 Jul 2018 22:52:49 +1000
Subject: [PATCH 3/5] Engine: add g_debug statements in state changing
interface functions
---
engine/dconf-engine.c | 10 +++++++++-
gsettings/dconfsettingsbackend.c | 1 +
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/engine/dconf-engine.c b/engine/dconf-engine.c
index 1963c34..2911724 100644
--- a/engine/dconf-engine.c
+++ b/engine/dconf-engine.c
@@ -928,11 +928,13 @@ dconf_engine_watch_established (DConfEngine *engine,
* everything under the path being watched changed. This case is
* very rare, anyway...
*/
+ g_debug ("SHM invalidated while establishing subscription to %s - signalling change", ow->path);
dconf_engine_change_notify (engine, ow->path, changes, NULL, FALSE, NULL, engine->user_data);
}
guint num_establishing = dconf_engine_count_subscriptions (engine->establishing,
ow->path);
+ g_debug ("watch_established: \"%s\" (establishing: %d)", ow->path, num_establishing);
if (num_establishing > 0)
// Subscription(s): establishing -> active
dconf_engine_move_subscriptions (engine->establishing,
@@ -948,6 +950,7 @@ dconf_engine_watch_fast (DConfEngine *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);
if (num_active > 0)
// Subscription: inactive -> active
dconf_engine_inc_subscriptions (engine->active, path);
@@ -1000,6 +1003,7 @@ dconf_engine_unwatch_fast (DConfEngine *engine,
guint num_active = dconf_engine_count_subscriptions (engine->active, path);
guint num_establishing = dconf_engine_count_subscriptions (engine->establishing, path);
gint i;
+ g_debug ("unwatch_fast: \"%s\" (active: %d, establishing: %d)", path, num_active, num_establishing);
// Client code cannot unsubscribe if it is not subscribed
g_assert (num_active > 0 || num_establishing > 0);
@@ -1056,6 +1060,7 @@ dconf_engine_watch_sync (DConfEngine *engine,
const gchar *path)
{
guint num_active = dconf_engine_inc_subscriptions (engine->active, path);
+ g_debug ("watch_sync: \"%s\" (active: %d)", path, num_active - 1);
if (num_active == 1)
dconf_engine_handle_match_rule_sync (engine, "AddMatch", path);
}
@@ -1065,6 +1070,7 @@ dconf_engine_unwatch_sync (DConfEngine *engine,
const gchar *path)
{
guint num_active = dconf_engine_dec_subscriptions (engine->active, path);
+ g_debug ("unwatch_sync: \"%s\" (active: %d)", path, num_active + 1);
if (num_active == 0)
dconf_engine_handle_match_rule_sync (engine, "RemoveMatch", path);
}
@@ -1274,7 +1280,7 @@ dconf_engine_change_fast (DConfEngine *engine,
GError **error)
{
GList *node;
-
+ g_debug ("change_fast");
if (dconf_changeset_is_empty (changeset))
return TRUE;
@@ -1341,6 +1347,7 @@ dconf_engine_change_sync (DConfEngine *engine,
GError **error)
{
GVariant *reply;
+ g_debug ("change_sync");
if (dconf_changeset_is_empty (changeset))
{
@@ -1519,6 +1526,7 @@ dconf_engine_has_outstanding (DConfEngine *engine)
void
dconf_engine_sync (DConfEngine *engine)
{
+ g_debug ("sync");
dconf_engine_lock_queues (engine);
while (!g_queue_is_empty (&engine->in_flight))
g_cond_wait (&engine->queue_cond, &engine->queue_lock);
diff --git a/gsettings/dconfsettingsbackend.c b/gsettings/dconfsettingsbackend.c
index 752e013..6c8179b 100644
--- a/gsettings/dconfsettingsbackend.c
+++ b/gsettings/dconfsettingsbackend.c
@@ -232,6 +232,7 @@ dconf_engine_change_notify (DConfEngine *engine,
{
GWeakRef *weak_ref = user_data;
DConfSettingsBackend *dcsb;
+ g_debug ("change_notify: %s", prefix);
dcsb = g_weak_ref_get (weak_ref);
--
2.20.1

View File

@ -0,0 +1,167 @@
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

View File

@ -0,0 +1,252 @@
From 8760833820e03a21900afda2d2f5785610c59ac9 Mon Sep 17 00:00:00 2001
From: Daniel Playfair Cal <daniel.playfair.cal@gmail.com>
Date: Sat, 28 Jul 2018 13:38:00 +1000
Subject: [PATCH 5/5] Engine: Add comprehensive unit tests for subscription
counting behaviour
Use g_assert_false instead of g_assert in unit tests
---
tests/engine.c | 206 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 206 insertions(+)
diff --git a/tests/engine.c b/tests/engine.c
index 038c04c..f2e57b2 100644
--- a/tests/engine.c
+++ b/tests/engine.c
@@ -1232,6 +1232,185 @@ test_watch_fast (void)
g_variant_unref (triv);
}
+static void
+test_watch_fast_simultaneous_subscriptions (void)
+{
+ /**
+ * Test that creating multiple subscriptions to the same path
+ * simultaneously (before receiving replies from D-Bus) only results in
+ * a single D-Bus match rule, and that it is removed at the right time.
+ */
+ DConfEngine *engine;
+ GvdbTable *table;
+ GVariant *triv;
+
+ /* Set up */
+ table = dconf_mock_gvdb_table_new ();
+ dconf_mock_gvdb_install ("/HOME/.config/dconf/user", table);
+ table = dconf_mock_gvdb_table_new ();
+ dconf_mock_gvdb_install ("/etc/dconf/db/site", table);
+
+ triv = g_variant_ref_sink (g_variant_new ("()"));
+
+ change_log = g_string_new (NULL);
+
+ engine = dconf_engine_new (SRCDIR "/profile/dos", NULL, NULL);
+
+
+ /* Subscribe to the same path 3 times. Both AddMatch results succeed
+ * (one for each source). There is only one for each source*path.
+ */
+ dconf_engine_watch_fast (engine, "/a/b/c");
+ dconf_engine_watch_fast (engine, "/a/b/c");
+ dconf_engine_watch_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 ();
+
+ /* Unsubscribe twice, after the AddMatch succeeds. There is still one
+ * active subscription, so no RemoveMatch request is sent. */
+ dconf_engine_unwatch_fast (engine, "/a/b/c");
+ dconf_engine_unwatch_fast (engine, "/a/b/c");
+
+ dconf_mock_dbus_assert_no_async ();
+
+ /* Unsubscribe once more. The number of active subscriptions falls to 0
+ * and the D-Bus match rule is removed */
+ 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 ();
+
+ /* The shm was not flagged at any point - so no change notifications
+ * should not have been sent */
+ g_assert_cmpstr (change_log->str, ==, "");
+
+ /* Clean up */
+ dconf_engine_unref (engine);
+ g_string_free (change_log, TRUE);
+ change_log = NULL;
+ g_variant_unref (triv);
+}
+
+static void
+test_watch_fast_successive_subscriptions (void)
+{
+ /**
+ * Test that subscribing to the same path multiple times successively
+ * (after waiting for any expected replies from D-Bus) results in only
+ * a single D-Bus match rule being created, and that it is created and
+ * destroyed at the right times.
+ */
+ DConfEngine *engine;
+ GvdbTable *table;
+ GVariant *triv;
+
+ /* Set up */
+ table = dconf_mock_gvdb_table_new ();
+ dconf_mock_gvdb_install ("/HOME/.config/dconf/user", table);
+ table = dconf_mock_gvdb_table_new ();
+ dconf_mock_gvdb_install ("/etc/dconf/db/site", table);
+
+ triv = g_variant_ref_sink (g_variant_new ("()"));
+
+ change_log = g_string_new (NULL);
+
+ engine = dconf_engine_new (SRCDIR "/profile/dos", NULL, NULL);
+
+ /* Subscribe to a path, and simulate a change to the database while the
+ * AddMatch request is in progress */
+ dconf_engine_watch_fast (engine, "/a/b/c");
+ dconf_mock_shm_flag ("user");
+ dconf_mock_dbus_async_reply (triv, NULL);
+ dconf_mock_dbus_async_reply (triv, NULL);
+
+ /* When the AddMatch requests succeeds, expect a change notification
+ * for the path */
+ dconf_mock_dbus_assert_no_async ();
+ g_assert_cmpstr (change_log->str, ==, "/a/b/c:1::nil;");
+
+ /* Subscribe to a path twice again, and simulate a change to the
+ * database */
+ dconf_engine_watch_fast (engine, "/a/b/c");
+ dconf_engine_watch_fast (engine, "/a/b/c");
+ dconf_mock_shm_flag ("user");
+
+ /* There was already a match rule in place, so there should be no D-Bus
+ * requests and no change notifications */
+ dconf_mock_dbus_assert_no_async ();
+ g_assert_cmpstr (change_log->str, ==, "/a/b/c:1::nil;");
+
+ /* Unsubscribe */
+ dconf_engine_unwatch_fast (engine, "/a/b/c");
+ dconf_engine_unwatch_fast (engine, "/a/b/c");
+ 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 ();
+
+
+ /* Clean up */
+ dconf_engine_unref (engine);
+ g_string_free (change_log, TRUE);
+ change_log = NULL;
+ g_variant_unref (triv);
+}
+
+static void
+test_watch_fast_short_lived_subscriptions (void)
+{
+ /**
+ * Test that subscribing and then immediately unsubscribing (without
+ * waiting for replies from D-Bus) multiple times to the same path
+ * correctly triggers D-Bus requests and change notifications in cases
+ * where the D-Bus match rule was not in place when the database was
+ * changed.
+ */
+ DConfEngine *engine;
+ GvdbTable *table;
+ GVariant *triv;
+
+ /* Set up */
+ table = dconf_mock_gvdb_table_new ();
+ dconf_mock_gvdb_install ("/HOME/.config/dconf/user", table);
+ table = dconf_mock_gvdb_table_new ();
+ dconf_mock_gvdb_install ("/etc/dconf/db/site", table);
+
+ triv = g_variant_ref_sink (g_variant_new ("()"));
+
+ change_log = g_string_new (NULL);
+
+ engine = dconf_engine_new (SRCDIR "/profile/dos", NULL, NULL);
+
+ /* Subscribe to a path twice, and simulate a change to the database
+ * while the AddMatch request is in progress */
+ dconf_engine_watch_fast (engine, "/a/b/c");
+ dconf_engine_watch_fast (engine, "/a/b/c");
+ dconf_mock_shm_flag ("user");
+ dconf_engine_unwatch_fast (engine, "/a/b/c");
+ 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_async_reply (triv, NULL);
+ dconf_mock_dbus_async_reply (triv, NULL);
+ dconf_mock_dbus_assert_no_async ();
+
+ /* When the AddMatch requests succeed, expect a change notification
+ * to have been sent for the path, even though the client has since
+ * unsubscribed. */
+ g_assert_cmpstr (change_log->str, ==, "/a/b/c:1::nil;");
+
+
+ /* Clean up */
+ dconf_engine_unref (engine);
+ g_string_free (change_log, TRUE);
+ change_log = NULL;
+ g_variant_unref (triv);
+}
+
static const gchar *match_request_type;
static gboolean got_match_request[5];
@@ -1270,13 +1449,37 @@ test_watch_sync (void)
engine = dconf_engine_new (SRCDIR "/profile/dos", NULL, NULL);
match_request_type = "AddMatch";
+
+ /* A match rule should be added when the first subscription is established */
dconf_engine_watch_sync (engine, "/a/b/c");
g_assert (got_match_request[G_BUS_TYPE_SESSION]);
g_assert (got_match_request[G_BUS_TYPE_SYSTEM]);
got_match_request[G_BUS_TYPE_SESSION] = FALSE;
got_match_request[G_BUS_TYPE_SYSTEM] = FALSE;
+ /* The match rule is now already in place, so more are not needed */
+ dconf_engine_watch_sync (engine, "/a/b/c");
+ g_assert_false (got_match_request[G_BUS_TYPE_SESSION]);
+ g_assert_false (got_match_request[G_BUS_TYPE_SYSTEM]);
+
+ dconf_engine_watch_sync (engine, "/a/b/c");
+ g_assert_false (got_match_request[G_BUS_TYPE_SESSION]);
+ g_assert_false (got_match_request[G_BUS_TYPE_SYSTEM]);
+
match_request_type = "RemoveMatch";
+
+ /* There are 3 subscriptions, so removing 2 should not remove
+ * the match rule */
+ dconf_engine_unwatch_sync (engine, "/a/b/c");
+ g_assert_false (got_match_request[G_BUS_TYPE_SESSION]);
+ g_assert_false (got_match_request[G_BUS_TYPE_SYSTEM]);
+
+ dconf_engine_unwatch_sync (engine, "/a/b/c");
+ g_assert_false (got_match_request[G_BUS_TYPE_SESSION]);
+ g_assert_false (got_match_request[G_BUS_TYPE_SYSTEM]);
+
+ /* The match rule should be removed when the last subscription is
+ * removed */
dconf_engine_unwatch_sync (engine, "/a/b/c");
g_assert (got_match_request[G_BUS_TYPE_SESSION]);
g_assert (got_match_request[G_BUS_TYPE_SYSTEM]);
@@ -1773,6 +1976,9 @@ main (int argc, char **argv)
g_test_add_func ("/engine/sources/service", test_service_source);
g_test_add_func ("/engine/read", test_read);
g_test_add_func ("/engine/watch/fast", test_watch_fast);
+ g_test_add_func ("/engine/watch/fast/simultaneous", test_watch_fast_simultaneous_subscriptions);
+ g_test_add_func ("/engine/watch/fast/successive", test_watch_fast_successive_subscriptions);
+ g_test_add_func ("/engine/watch/fast/short_lived", test_watch_fast_short_lived_subscriptions);
g_test_add_func ("/engine/watch/sync", test_watch_sync);
g_test_add_func ("/engine/change/fast", test_change_fast);
g_test_add_func ("/engine/change/sync", test_change_sync);
--
2.20.1

View File

@ -0,0 +1,77 @@
From 6a6797446f13378035a2700253546b524d629c8a Mon Sep 17 00:00:00 2001
From: Marek Kasik <mkasik@redhat.com>
Date: Tue, 10 Jul 2018 18:29:16 +0200
Subject: [PATCH] Check mtimes of files when updating databases
Do not check just mtimes of directories in /etc/dconf/db/
but also mtimes of the files in those directories
to catch all modifications in them.
https://bugzilla.gnome.org/show_bug.cgi?id=708258
---
bin/dconf-update.vala | 41 ++++++++++++++++++++++++++++++++++-------
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/bin/dconf-update.vala b/bin/dconf-update.vala
index d452092..5aac6c7 100644
--- a/bin/dconf-update.vala
+++ b/bin/dconf-update.vala
@@ -162,21 +162,48 @@ Gvdb.HashTable read_directory (string dirname) throws GLib.Error {
return table;
}
+time_t get_directory_mtime (string dirname, Posix.Stat dir_buf) throws GLib.Error {
+ Posix.Stat lockdir_buf;
+ Posix.Stat file_buf;
+ time_t dir_mtime = dir_buf.st_mtime;
+
+ var files = list_directory (dirname, Posix.S_IFREG);
+ files.sort (strcmp);
+ files.reverse ();
+
+ foreach (var filename in files) {
+ if (Posix.stat (filename, out file_buf) == 0 && file_buf.st_mtime > dir_mtime)
+ dir_mtime = file_buf.st_mtime;
+ }
+
+ if (Posix.stat (dirname + "/locks", out lockdir_buf) == 0 && Posix.S_ISDIR (lockdir_buf.st_mode)) {
+ if (lockdir_buf.st_mtime > dir_mtime) {
+ // if the lock directory has been updated more recently then consider its timestamp instead
+ dir_mtime = lockdir_buf.st_mtime;
+ }
+
+ files = list_directory (dirname + "/locks", Posix.S_IFREG);
+ files.sort (strcmp);
+ files.reverse ();
+
+ foreach (var filename in files) {
+ if (Posix.stat (filename, out file_buf) == 0 && file_buf.st_mtime > dir_mtime)
+ dir_mtime = file_buf.st_mtime;
+ }
+ }
+
+ return dir_mtime;
+}
+
void maybe_update_from_directory (string dirname) throws GLib.Error {
Posix.Stat dir_buf;
if (Posix.stat (dirname, out dir_buf) == 0 && Posix.S_ISDIR (dir_buf.st_mode)) {
- Posix.Stat lockdir_buf;
Posix.Stat file_buf;
var filename = dirname.substring (0, dirname.length - 2);
- if (Posix.stat (dirname + "/locks", out lockdir_buf) == 0 && lockdir_buf.st_mtime > dir_buf.st_mtime) {
- // if the lock directory has been updated more recently then consider its timestamp instead
- dir_buf.st_mtime = lockdir_buf.st_mtime;
- }
-
- if (Posix.stat (filename, out file_buf) == 0 && file_buf.st_mtime > dir_buf.st_mtime) {
+ if (Posix.stat (filename, out file_buf) == 0 && file_buf.st_mtime > get_directory_mtime (dirname, dir_buf)) {
return;
}
--
2.17.1

View File

@ -1,23 +0,0 @@
diff --git a/engine/dconf-engine-source-user.c b/engine/dconf-engine-source-user.c
index 1657875..e4f8786 100644
--- a/engine/dconf-engine-source-user.c
+++ b/engine/dconf-engine-source-user.c
@@ -39,11 +39,17 @@ dconf_engine_source_user_open_gvdb (const gchar *name)
{
GvdbTable *table;
gchar *filename;
+ const gchar *override;
+
+ override = g_getenv ("DCONF_USER_CONFIG_DIR");
+ if (override == NULL)
+ filename = g_build_filename (g_get_user_config_dir (), "dconf", name, NULL);
+ else
+ filename = g_build_filename (g_get_home_dir (), override, name, NULL);
/* This can fail in the normal case of the user not having any
* settings. That's OK and it shouldn't be considered as an error.
*/
- filename = g_build_filename (g_get_user_config_dir (), "dconf", name, NULL);
table = gvdb_table_new (filename, FALSE, NULL);
g_free (filename);

View File

@ -1,32 +1,32 @@
%define glib2_version 2.44.0 %define glib2_version 2.44.0
Name: dconf Name: dconf
Version: 0.40.0 Version: 0.28.0
Release: 6%{?dist} Release: 4%{?dist}
Summary: A configuration system Summary: A configuration system
License: LGPLv2+ and GPLv2+ and GPLv3+ License: LGPLv2+ and GPLv2+ and GPLv3+
URL: https://wiki.gnome.org/Projects/dconf URL: http://live.gnome.org/dconf
Source0: https://download.gnome.org/sources/dconf/0.40/dconf-%{version}.tar.xz Source0: http://download.gnome.org/sources/dconf/0.28/dconf-%{version}.tar.xz
Patch1: dconf-override.patch Patch0: dconf-0.28.0-db-mtime.patch
Patch2: dconf-0.40.0-permissions.patch Patch1: 0001-Engine-track-in-progress-watch-handles-to-avoid-spur.patch
Patch2: 0001-Engine-add-some-missing-objects-to-dconf_engine_unre.patch
Patch3: 0002-Engine-extend-subscriptions-state-to-account-for-mul.patch
Patch4: 0003-Engine-add-g_debug-statements-in-state-changing-inte.patch
Patch5: 0004-Engine-Add-locks-around-access-to-subscription-count.patch
Patch6: 0005-Engine-Add-comprehensive-unit-tests-for-subscription.patch
Patch7: 0001-Engine-Change-overflow-thresholds-in-subscription-co.patch
Patch8: dconf-0.28.0-permissions.patch
BuildRequires: bash-completion
BuildRequires: dbus-devel
BuildRequires: gcc
BuildRequires: glib2-devel >= %{glib2_version} BuildRequires: glib2-devel >= %{glib2_version}
BuildRequires: gtk-doc BuildRequires: gtk-doc
BuildRequires: meson BuildRequires: meson
BuildRequires: systemd
BuildRequires: vala BuildRequires: vala
Requires: dbus Requires: dbus
Requires: glib2%{?_isa} >= %{glib2_version} Requires: glib2%{?_isa} >= %{glib2_version}
# gvdb code is intended to be copy-pasted into projects
Provides: bundled(gvdb)
%description %description
dconf is a low-level configuration system. Its main purpose is to provide a dconf is a low-level configuration system. Its main purpose is to provide a
backend to the GSettings API in GLib. backend to the GSettings API in GLib.
@ -43,7 +43,7 @@ development using dconf.
%autosetup -p1 %autosetup -p1
%build %build
%meson -Dgtk_doc=true %meson -Denable-gtk-doc=true
%meson_build %meson_build
%install %install
@ -63,16 +63,7 @@ mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/dconf/db/site.d/locks
mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/dconf/db/distro.d/locks mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/dconf/db/distro.d/locks
%posttrans %posttrans
%{_bindir}/dconf update dconf update
%post
%systemd_user_post dconf.service
%preun
%systemd_user_preun dconf.service
%postun
%systemd_user_postun_with_restart dconf.service
%files %files
%license COPYING %license COPYING
@ -85,17 +76,16 @@ mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/dconf/db/distro.d/locks
%dir %{_sysconfdir}/dconf/db/distro.d %dir %{_sysconfdir}/dconf/db/distro.d
%dir %{_sysconfdir}/dconf/db/distro.d/locks %dir %{_sysconfdir}/dconf/db/distro.d/locks
%dir %{_sysconfdir}/dconf/profile %dir %{_sysconfdir}/dconf/profile
%config(noreplace) %{_sysconfdir}/dconf/profile/user
%{_libdir}/gio/modules/libdconfsettings.so %{_libdir}/gio/modules/libdconfsettings.so
%{_libexecdir}/dconf-service %{_libexecdir}/dconf-service
%{_datadir}/dbus-1/services/ca.desrt.dconf.service %{_datadir}/dbus-1/services/ca.desrt.dconf.service
%{_bindir}/dconf %{_bindir}/dconf
%{_libdir}/libdconf.so.1* %{_libdir}/libdconf.so.1*
%{_datadir}/bash-completion/completions/dconf %{_datadir}/bash-completion/completions/dconf
%{_mandir}/man1/dconf-service.1* %{_mandir}/man1/dconf-service.1.gz
%{_mandir}/man1/dconf.1* %{_mandir}/man1/dconf.1.gz
%{_mandir}/man7/dconf.7* %{_mandir}/man7/dconf.7.gz
%{_userunitdir}/dconf.service %config(noreplace) %{_sysconfdir}/dconf/profile/user
%files devel %files devel
%{_includedir}/dconf %{_includedir}/dconf
@ -107,87 +97,19 @@ mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/dconf/db/distro.d/locks
%{_datadir}/vala %{_datadir}/vala
%changelog %changelog
* Mon Aug 09 2021 Mohan Boddu <mboddu@redhat.com> - 0.40.0-6 * Mon Nov 23 2020 Marek Kasik <mkasik@redhat.com> - 0.28.0-4
- Rebuilt for IMA sigs, glibc 2.34, aarch64 flags - Restore permissions on updated database
Related: rhbz#1991688 - Resolves: #1879722
* Tue Jun 22 2021 Marek Kasik <mkasik@redhat.com> - 0.40.0-5 * Fri Jan 18 2019 Olivier Fourdan <ofourdan@redhat.com> - 0.28.0-3
- Backport patch for permissions restore - Backport fix for emission of spurious changed signals
- Indicate bundle of GVDB - for all keys at once crashing gnome-shell
- Resolves: #1974362 - Resolves: #1666176
* Mon Apr 19 2021 Kalev Lember <klember@redhat.com> - 0.40.0-4 * Wed Jul 25 2018 Marek Kasik <mkasik@redhat.com> - 0.28.0-2
- Use full path for dconf in posttrans script (#1950829) - Check mtimes of files in /etc/dconf/db/*.d/ directories
- when running "dconf update"
* Thu Apr 15 2021 Mohan Boddu <mboddu@redhat.com> - 0.40.0-3 - Related: #1570569
- Rebuilt for RHEL 9 BETA on Apr 15th 2021. Related: rhbz#1947937
* Wed Mar 31 2021 Kalev Lember <klember@redhat.com> - 0.40.0-2
- Avoid hardcoding man page compression
* Mon Mar 15 2021 Kalev Lember <klember@redhat.com> - 0.40.0-1
- Update to 0.40.0
* Thu Feb 18 2021 Kalev Lember <klember@redhat.com> - 0.39.1-1
- Update to 0.39.1
* Tue Jan 26 2021 Fedora Release Engineering <releng@fedoraproject.org> - 0.38.0-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_34_Mass_Rebuild
* Sun Sep 13 2020 Kalev Lember <klember@redhat.com> - 0.38.0-1
- Update to 0.38.0
* Mon Jul 27 2020 Fedora Release Engineering <releng@fedoraproject.org> - 0.36.0-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_33_Mass_Rebuild
* Tue Mar 10 2020 Kalev Lember <klember@redhat.com> - 0.36.0-1
- Update to 0.36.0
* Tue Jan 28 2020 Fedora Release Engineering <releng@fedoraproject.org> - 0.35.1-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_32_Mass_Rebuild
* Tue Jan 07 2020 Kalev Lember <klember@redhat.com> - 0.35.1-1
- Update to 0.35.1
* Tue Sep 10 2019 Kalev Lember <klember@redhat.com> - 0.34.0-1
- Update to 0.34.0
* Tue Aug 20 2019 Kalev Lember <klember@redhat.com> - 0.33.2-1
- Update to 0.33.2
* Wed Jul 24 2019 Fedora Release Engineering <releng@fedoraproject.org> - 0.33.1-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_31_Mass_Rebuild
* Tue Jul 16 2019 Kalev Lember <klember@redhat.com> - 0.33.1-1
- Update to 0.33.1
* Mon Mar 11 2019 Kalev Lember <klember@redhat.com> - 0.32.0-1
- Update to 0.32.0
* Fri Mar 08 2019 Kalev Lember <klember@redhat.com> - 0.31.92-1
- Update to 0.31.92
* Mon Feb 04 2019 Kalev Lember <klember@redhat.com> - 0.31.2-1
- Update to 0.31.2
* Thu Jan 31 2019 Fedora Release Engineering <releng@fedoraproject.org> - 0.31.1-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_30_Mass_Rebuild
* Mon Jan 07 2019 Kalev Lember <klember@redhat.com> - 0.31.1-1
- Update to 0.31.1
- Update project URLs
* Fri Oct 26 2018 Kalev Lember <klember@redhat.com> - 0.30.1-1
- Update to 0.30.1
* Wed Sep 05 2018 Kalev Lember <klember@redhat.com> - 0.30.0-1
- Update to 0.30.0
* Tue Aug 21 2018 Owen Taylor <otaylor@redhat.com> - 0.28.0-3
- Add a patch to enable DCONF_USER_CONFIG_DIR environment variable
* Thu Jul 12 2018 Fedora Release Engineering <releng@fedoraproject.org> - 0.28.0-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_29_Mass_Rebuild
* Tue Mar 13 2018 Kalev Lember <klember@redhat.com> - 0.28.0-1 * Tue Mar 13 2018 Kalev Lember <klember@redhat.com> - 0.28.0-1
- Update to 0.28.0 - Update to 0.28.0