[evolution-data-server] EDataBookFactory: Rewrite client connection tracking.

[Matthew Barnes <mbarnes src gnome org>]

commit 26d1e871576e35015b2e243805922d7fc7fe8bf6
Author: Matthew Barnes <mbarnes redhat com>
Date:   Wed Mar 20 08:39:03 2013 -0400

    EDataBookFactory: Rewrite client connection tracking.
    
    Keep a hash table of client bus names mapped to a set of EBookBackend
    references.  When the backend emits a closed() signal with the client's
    bus name, we remove that EBookBackend reference from the data structure.
    
    Also keep a hash table of client bus names we're watching so that if a
    bus name vanishes we can remove all EBookBackend references associated
    with it.

 addressbook/libedata-book/e-data-book-factory.c |  295 ++++++++++++++++-------
 1 files changed, 203 insertions(+), 92 deletions(-)
---
diff --git a/addressbook/libedata-book/e-data-book-factory.c b/addressbook/libedata-book/e-data-book-factory.c
index 37a79e2..1a3bff2 100644
--- a/addressbook/libedata-book/e-data-book-factory.c
+++ b/addressbook/libedata-book/e-data-book-factory.c
@@ -44,9 +44,15 @@ struct _EDataBookFactoryPrivate {
        ESourceRegistry *registry;
        EDBusAddressBookFactory *dbus_factory;
 
-       GMutex connections_lock;
-       /* This is a hash of client addresses to GList* of EDataBooks */
+       /* This is a hash table of client bus names to an array of
+        * EBookBackend references; one for every connection opened. */
        GHashTable *connections;
+       GMutex connections_lock;
+
+       /* This is a hash table of client bus names being watched.
+        * The value is the watcher ID for g_bus_unwatch_name(). */
+       GHashTable *watched_names;
+       GMutex watched_names_lock;
 };
 
 enum {
@@ -66,6 +72,156 @@ G_DEFINE_TYPE_WITH_CODE (
                G_TYPE_INITABLE,
                e_data_book_factory_initable_init))
 
+static void
+watched_names_value_free (gpointer value)
+{
+       g_bus_unwatch_name (GPOINTER_TO_UINT (value));
+}
+
+static void
+data_book_factory_connections_add (EDataBookFactory *factory,
+                                   const gchar *name,
+                                   EBookBackend *backend)
+{
+       GHashTable *connections;
+       GPtrArray *array;
+
+       g_return_if_fail (name != NULL);
+       g_return_if_fail (backend != NULL);
+
+       g_mutex_lock (&factory->priv->connections_lock);
+
+       connections = factory->priv->connections;
+
+       if (g_hash_table_size (connections) == 0)
+               e_dbus_server_hold (E_DBUS_SERVER (factory));
+
+       array = g_hash_table_lookup (connections, name);
+
+       if (array == NULL) {
+               array = g_ptr_array_new_with_free_func (
+                       (GDestroyNotify) g_object_unref);
+               g_hash_table_insert (
+                       connections, g_strdup (name), array);
+       }
+
+       g_ptr_array_add (array, g_object_ref (backend));
+
+       g_mutex_unlock (&factory->priv->connections_lock);
+}
+
+static gboolean
+data_book_factory_connections_remove (EDataBookFactory *factory,
+                                      const gchar *name,
+                                      EBookBackend *backend)
+{
+       GHashTable *connections;
+       GPtrArray *array;
+       gboolean removed = FALSE;
+
+       /* If backend is NULL, we remove all backends for name. */
+       g_return_val_if_fail (name != NULL, FALSE);
+
+       g_mutex_lock (&factory->priv->connections_lock);
+
+       connections = factory->priv->connections;
+       array = g_hash_table_lookup (connections, name);
+
+       if (array != NULL) {
+               if (backend != NULL) {
+                       removed = g_ptr_array_remove_fast (array, backend);
+               } else if (array->len > 0) {
+                       g_ptr_array_set_size (array, 0);
+                       removed = TRUE;
+               }
+
+               if (array->len == 0)
+                       g_hash_table_remove (connections, name);
+
+               if (g_hash_table_size (connections) == 0)
+                       e_dbus_server_release (E_DBUS_SERVER (factory));
+       }
+
+       g_mutex_unlock (&factory->priv->connections_lock);
+
+       return removed;
+}
+
+static void
+data_book_factory_connections_remove_all (EDataBookFactory *factory)
+{
+       GHashTable *connections;
+
+       g_mutex_lock (&factory->priv->connections_lock);
+
+       connections = factory->priv->connections;
+
+       if (g_hash_table_size (connections) > 0) {
+               g_hash_table_remove_all (connections);
+               e_dbus_server_release (E_DBUS_SERVER (factory));
+       }
+
+       g_mutex_unlock (&factory->priv->connections_lock);
+}
+
+static void
+data_book_factory_name_vanished_cb (GDBusConnection *connection,
+                                    const gchar *name,
+                                    gpointer user_data)
+{
+       GWeakRef *weak_ref = user_data;
+       EDataBookFactory *factory;
+
+       factory = g_weak_ref_get (weak_ref);
+
+       if (factory != NULL) {
+               g_mutex_lock (&factory->priv->watched_names_lock);
+               g_hash_table_remove (factory->priv->watched_names, name);
+               g_mutex_unlock (&factory->priv->watched_names_lock);
+
+               data_book_factory_connections_remove (factory, name, NULL);
+
+               g_object_unref (factory);
+       }
+}
+
+static void
+data_book_factory_watched_names_add (EDataBookFactory *factory,
+                                     GDBusConnection *connection,
+                                     const gchar *name)
+{
+       GHashTable *watched_names;
+
+       g_return_if_fail (name != NULL);
+
+       g_mutex_lock (&factory->priv->watched_names_lock);
+
+       watched_names = factory->priv->watched_names;
+
+       if (!g_hash_table_contains (watched_names, name)) {
+               guint watcher_id;
+
+               /* The g_bus_watch_name() documentation says one of the two
+                * callbacks are guaranteed to be invoked after calling the
+                * function.  But which one is determined asynchronously so
+                * there should be no chance of the name vanished callback
+                * deadlocking with us when it tries to acquire the lock. */
+               watcher_id = g_bus_watch_name_on_connection (
+                       connection, name,
+                       G_BUS_NAME_WATCHER_FLAGS_NONE,
+                       (GBusNameAppearedCallback) NULL,
+                       data_book_factory_name_vanished_cb,
+                       e_weak_ref_new (factory),
+                       (GDestroyNotify) e_weak_ref_free);
+
+               g_hash_table_insert (
+                       watched_names, g_strdup (name),
+                       GUINT_TO_POINTER (watcher_id));
+       }
+
+       g_mutex_unlock (&factory->priv->watched_names_lock);
+}
+
 static EBackend *
 data_book_factory_ref_backend (EDataFactory *factory,
                                ESource *source,
@@ -114,37 +270,11 @@ construct_book_factory_path (void)
 }
 
 static void
-book_freed_cb (EDataBookFactory *factory,
-               GObject *dead)
+data_book_factory_closed_cb (EBookBackend *backend,
+                             const gchar *sender,
+                             EDataBookFactory *factory)
 {
-       EDataBookFactoryPrivate *priv = factory->priv;
-       GHashTableIter iter;
-       gpointer hkey, hvalue;
-
-       d (g_debug ("in factory %p (%p) is dead", factory, dead));
-
-       g_mutex_lock (&priv->connections_lock);
-
-       g_hash_table_iter_init (&iter, priv->connections);
-       while (g_hash_table_iter_next (&iter, &hkey, &hvalue)) {
-               GList *books = hvalue;
-
-               if (g_list_find (books, dead)) {
-                       books = g_list_remove (books, dead);
-                       if (books != NULL)
-                               g_hash_table_insert (
-                                       priv->connections,
-                                       g_strdup (hkey), books);
-                       else
-                               g_hash_table_remove (priv->connections, hkey);
-
-                       break;
-               }
-       }
-
-       g_mutex_unlock (&priv->connections_lock);
-
-       e_dbus_server_release (E_DBUS_SERVER (factory));
+       data_book_factory_connections_remove (factory, sender, backend);
 }
 
 static gchar *
@@ -154,12 +284,11 @@ data_book_factory_open (EDataBookFactory *factory,
                         const gchar *uid,
                         GError **error)
 {
-       EDataBook *book;
+       EDataBook *data_book;
        EBackend *backend;
        ESourceRegistry *registry;
        ESource *source;
        gchar *object_path;
-       GList *list;
 
        if (uid == NULL || *uid == '\0') {
                g_set_error (
@@ -188,37 +317,38 @@ data_book_factory_open (EDataBookFactory *factory,
        if (backend == NULL)
                return NULL;
 
-       e_dbus_server_hold (E_DBUS_SERVER (factory));
-
        object_path = construct_book_factory_path ();
 
-       book = e_data_book_new (
+       data_book = e_data_book_new (
                E_BOOK_BACKEND (backend),
                connection, object_path, error);
 
-       if (book != NULL) {
-               e_book_backend_add_client (E_BOOK_BACKEND (backend), book);
+       if (data_book != NULL) {
+               e_book_backend_add_client (E_BOOK_BACKEND (backend), data_book);
 
-               g_object_weak_ref (
-                       G_OBJECT (book), (GWeakNotify)
-                       book_freed_cb, factory);
+               data_book_factory_watched_names_add (
+                       factory, connection, sender);
 
-               /* Update the hash of open connections. */
-               g_mutex_lock (&factory->priv->connections_lock);
-               list = g_hash_table_lookup (
-                       factory->priv->connections, sender);
-               list = g_list_prepend (list, book);
-               g_hash_table_insert (
-                       factory->priv->connections,
-                       g_strdup (sender), list);
-               g_mutex_unlock (&factory->priv->connections_lock);
+               g_signal_connect_object (
+                       backend, "closed",
+                       G_CALLBACK (data_book_factory_closed_cb),
+                       factory, 0);
 
        } else {
                g_free (object_path);
                object_path = NULL;
        }
 
-       g_object_unref (backend);
+       if (data_book != NULL) {
+               /* A client may create multiple EClient instances for the
+                * same ESource, each of which calls close() individually.
+                * So we must track each and every connection made. */
+               data_book_factory_connections_add (
+                       factory, sender, E_BOOK_BACKEND (backend));
+       }
+
+       g_clear_object (&data_book);
+       g_clear_object (&backend);
 
        return object_path;
 }
@@ -253,19 +383,6 @@ data_book_factory_handle_open_address_book_cb (EDBusAddressBookFactory *interfac
 }
 
 static void
-remove_data_book_cb (EDataBook *data_book)
-{
-       EBookBackend *backend;
-
-       g_return_if_fail (data_book != NULL);
-
-       backend = e_data_book_get_backend (data_book);
-       e_book_backend_remove_client (backend, data_book);
-
-       g_object_unref (data_book);
-}
-
-static void
 data_book_factory_get_property (GObject *object,
                                 guint property_id,
                                 GValue *value,
@@ -300,7 +417,10 @@ data_book_factory_dispose (GObject *object)
                priv->dbus_factory = NULL;
        }
 
-       /* Chain up to parent's finalize() method. */
+       g_hash_table_remove_all (priv->connections);
+       g_hash_table_remove_all (priv->watched_names);
+
+       /* Chain up to parent's dispose() method. */
        G_OBJECT_CLASS (e_data_book_factory_parent_class)->dispose (object);
 }
 
@@ -312,9 +432,11 @@ data_book_factory_finalize (GObject *object)
        priv = E_DATA_BOOK_FACTORY_GET_PRIVATE (object);
 
        g_hash_table_destroy (priv->connections);
-
        g_mutex_clear (&priv->connections_lock);
 
+       g_hash_table_destroy (priv->watched_names);
+       g_mutex_clear (&priv->watched_names_lock);
+
        /* Chain up to parent's finalize() method. */
        G_OBJECT_CLASS (e_data_book_factory_parent_class)->finalize (object);
 }
@@ -350,30 +472,11 @@ static void
 data_book_factory_bus_name_lost (EDBusServer *server,
                                  GDBusConnection *connection)
 {
-       EDataBookFactoryPrivate *priv;
-       GList *list = NULL;
-       gchar *key;
-
-       priv = E_DATA_BOOK_FACTORY_GET_PRIVATE (server);
+       EDataBookFactory *factory;
 
-       g_mutex_lock (&priv->connections_lock);
-
-       while (g_hash_table_lookup_extended (
-               priv->connections,
-               ADDRESS_BOOK_DBUS_SERVICE_NAME,
-               (gpointer) &key, (gpointer) &list)) {
-               GList *copy;
-
-               /* this should trigger the book's weak ref notify
-                * function, which will remove it from the list before
-                * it's freed, and will remove the connection from
-                * priv->connections once they're all gone */
-               copy = g_list_copy (list);
-               g_list_foreach (copy, (GFunc) remove_data_book_cb, NULL);
-               g_list_free (copy);
-       }
+       factory = E_DATA_BOOK_FACTORY (server);
 
-       g_mutex_unlock (&priv->connections_lock);
+       data_book_factory_connections_remove_all (factory);
 
        /* Chain up to parent's bus_name_lost() method. */
        E_DBUS_SERVER_CLASS (e_data_book_factory_parent_class)->
@@ -472,11 +575,19 @@ e_data_book_factory_init (EDataBookFactory *factory)
                G_CALLBACK (data_book_factory_handle_open_address_book_cb),
                factory);
 
-       g_mutex_init (&factory->priv->connections_lock);
        factory->priv->connections = g_hash_table_new_full (
-               g_str_hash, g_str_equal,
+               (GHashFunc) g_str_hash,
+               (GEqualFunc) g_str_equal,
+               (GDestroyNotify) g_free,
+               (GDestroyNotify) g_ptr_array_unref);
+
+       g_mutex_init (&factory->priv->connections_lock);
+
+       factory->priv->watched_names = g_hash_table_new_full (
+               (GHashFunc) g_str_hash,
+               (GEqualFunc) g_str_equal,
                (GDestroyNotify) g_free,
-               (GDestroyNotify) NULL);
+               (GDestroyNotify) watched_names_value_free);
 }
 
 EDBusServer *