[libsoup/http2: 6/10] get rid of SoupSessionPrivate->conns hash

[Dan Winship <danw src gnome org>]

commit c568591bf9f285a1d849201aa5efc9b2fa8e9464
Author: Dan Winship <danw gnome org>
Date:   Tue Dec 10 12:06:34 2013 +0100

    get rid of SoupSessionPrivate->conns hash

 libsoup/soup-session-host.c |   81 +++++++++++++++++++++++++++++----------
 libsoup/soup-session-host.h |    3 +-
 libsoup/soup-session.c      |   87 ++++++++++++++++++++++---------------------
 3 files changed, 106 insertions(+), 65 deletions(-)
---
diff --git a/libsoup/soup-session-host.c b/libsoup/soup-session-host.c
index ba3e7f6..e959ddc 100644
--- a/libsoup/soup-session-host.c
+++ b/libsoup/soup-session-host.c
@@ -19,6 +19,8 @@
 G_DEFINE_TYPE (SoupSessionHost, soup_session_host, G_TYPE_OBJECT)
 
 typedef struct {
+       GMutex       mutex;
+
        SoupURI     *uri;
        SoupAddress *addr;
 
@@ -47,6 +49,9 @@ static guint signals[LAST_SIGNAL] = { 0 };
 static void
 soup_session_host_init (SoupSessionHost *host)
 {
+       SoupSessionHostPrivate *priv = SOUP_SESSION_HOST_GET_PRIVATE (host);
+
+       g_mutex_init (&priv->mutex);
 }
 
 static void
@@ -64,6 +69,8 @@ soup_session_host_finalize (GObject *object)
        soup_uri_free (priv->uri);
        g_object_unref (priv->addr);
 
+       g_mutex_clear (&priv->mutex);
+
        G_OBJECT_CLASS (soup_session_host_parent_class)->finalize (object);
 }
 
@@ -146,6 +153,30 @@ emit_unused (gpointer host)
        return FALSE;
 }
 
+static void
+connection_disconnected (SoupConnection *conn, gpointer host)
+{
+       SoupSessionHostPrivate *priv = SOUP_SESSION_HOST_GET_PRIVATE (host);
+
+       g_mutex_lock (&priv->mutex);
+
+       if (soup_connection_get_ssl_fallback (conn))
+               priv->ssl_fallback = TRUE;
+
+       priv->connections = g_slist_remove (priv->connections, conn);
+       priv->num_conns--;
+
+       if (priv->num_conns == 0) {
+               g_assert (priv->keep_alive_src == NULL);
+               priv->keep_alive_src = soup_add_timeout_reffed (soup_session_get_async_context 
(priv->session),
+                                                               HOST_KEEP_ALIVE,
+                                                               emit_unused,
+                                                               host);
+       }
+
+       g_mutex_unlock (&priv->mutex);
+}
+
 SoupConnection *
 soup_session_host_get_connection (SoupSessionHost *host,
                                  gboolean need_new_connection,
@@ -158,11 +189,14 @@ soup_session_host_get_connection (SoupSessionHost *host,
        int num_pending = 0;
        SoupSocketProperties *socket_props;
 
+       g_mutex_lock (&priv->mutex);
+
        for (conns = priv->connections; conns; conns = conns->next) {
                conn = conns->data;
 
                if (!need_new_connection && soup_connection_get_state (conn) == SOUP_CONNECTION_IDLE) {
                        soup_connection_set_state (conn, SOUP_CONNECTION_IN_USE);
+                       g_mutex_unlock (&priv->mutex);
                        return conn;
                } else if (soup_connection_get_state (conn) == SOUP_CONNECTION_CONNECTING)
                        num_pending++;
@@ -171,17 +205,21 @@ soup_session_host_get_connection (SoupSessionHost *host,
        /* Limit the number of pending connections; num_messages / 2
         * is somewhat arbitrary...
         */
-       if (num_pending > priv->num_messages / 2)
+       if (num_pending > priv->num_messages / 2) {
+               g_mutex_unlock (&priv->mutex);
                return NULL;
+       }
 
        if (priv->num_conns >= priv->max_conns) {
                if (need_new_connection)
                        *try_cleanup = TRUE;
+               g_mutex_unlock (&priv->mutex);
                return NULL;
        }
 
        if (at_max_conns) {
                *try_cleanup = TRUE;
+               g_mutex_unlock (&priv->mutex);
                return NULL;
        }
 
@@ -198,40 +236,41 @@ soup_session_host_get_connection (SoupSessionHost *host,
        priv->num_conns++;
        priv->connections = g_slist_prepend (priv->connections, conn);
 
+       g_signal_connect (conn, "disconnected",
+                         G_CALLBACK (connection_disconnected),
+                         host);
+
        if (priv->keep_alive_src) {
                g_source_destroy (priv->keep_alive_src);
                g_source_unref (priv->keep_alive_src);
                priv->keep_alive_src = NULL;
        }
 
+       g_mutex_unlock (&priv->mutex);
        return conn;
 }
 
-void
-soup_session_host_remove_connection (SoupSessionHost *host,
-                                    SoupConnection  *conn)
+int
+soup_session_host_get_num_connections (SoupSessionHost *host)
 {
-       SoupSessionHostPrivate *priv = SOUP_SESSION_HOST_GET_PRIVATE (host);
+       return SOUP_SESSION_HOST_GET_PRIVATE (host)->num_conns;
+}
 
-       if (soup_connection_get_ssl_fallback (conn))
-               priv->ssl_fallback = TRUE;
+GSList *
+soup_session_host_get_connections (SoupSessionHost *host)
+{
+       SoupSessionHostPrivate *priv = SOUP_SESSION_HOST_GET_PRIVATE (host);
+       GSList *conns, *c;
 
-       priv->connections = g_slist_remove (priv->connections, conn);
-       priv->num_conns--;
+       g_mutex_lock (&priv->mutex);
 
-       if (priv->num_conns == 0) {
-               g_assert (priv->keep_alive_src == NULL);
-               priv->keep_alive_src = soup_add_timeout_reffed (soup_session_get_async_context 
(priv->session),
-                                                               HOST_KEEP_ALIVE,
-                                                               emit_unused,
-                                                               host);
-       }
-}
+       conns = NULL;
+       for (c = priv->connections; c; c = c->next)
+               conns = g_slist_prepend (conns, g_object_ref (c->data));
+       conns = g_slist_reverse (conns);
 
-int
-soup_session_host_get_num_connections (SoupSessionHost *host)
-{
-       return SOUP_SESSION_HOST_GET_PRIVATE (host)->num_conns;
+       g_mutex_unlock (&priv->mutex);
+       return conns;
 }
 
 gboolean
diff --git a/libsoup/soup-session-host.h b/libsoup/soup-session-host.h
index fecd5d2..4f27b50 100644
--- a/libsoup/soup-session-host.h
+++ b/libsoup/soup-session-host.h
@@ -44,9 +44,8 @@ SoupConnection  *soup_session_host_get_connection      (SoupSessionHost      *ho
                                                        gboolean              need_new_connection,
                                                        gboolean              at_max_conns,
                                                        gboolean             *try_cleanup);
-void             soup_session_host_remove_connection   (SoupSessionHost      *host,
-                                                       SoupConnection       *conn);
 int              soup_session_host_get_num_connections (SoupSessionHost      *host);
+GSList          *soup_session_host_get_connections     (SoupSessionHost      *host);
 
 gboolean         soup_session_host_get_ssl_fallback    (SoupSessionHost      *host);
 void             soup_session_host_set_ssl_fallback    (SoupSessionHost      *host,
diff --git a/libsoup/soup-session.c b/libsoup/soup-session.c
index 03af7cd..1f4db04 100644
--- a/libsoup/soup-session.c
+++ b/libsoup/soup-session.c
@@ -87,7 +87,6 @@ typedef struct {
        GHashTable *features_cache;
 
        GHashTable *http_hosts, *https_hosts; /* char* -> SoupSessionHost */
-       GHashTable *conns; /* SoupConnection -> SoupSessionHost */
        guint num_conns;
        guint max_conns, max_conns_per_host;
        guint io_timeout, idle_timeout;
@@ -124,8 +123,7 @@ typedef struct {
 static void connection_state_changed (GObject *object, GParamSpec *param,
                                      gpointer user_data);
 static void connection_disconnected (SoupConnection *conn, gpointer user_data);
-static void drop_connection (SoupSession *session, SoupSessionHost *host,
-                            SoupConnection *conn);
+static void drop_connection (SoupSession *session, SoupConnection *conn);
 
 static void auth_manager_authenticate (SoupAuthManager *manager,
                                       SoupMessage *msg, SoupAuth *auth,
@@ -206,7 +204,6 @@ soup_session_init (SoupSession *session)
        priv->https_hosts = g_hash_table_new_full (soup_host_uri_hash,
                                                   soup_host_uri_equal,
                                                   NULL, g_object_unref);
-       priv->conns = g_hash_table_new (NULL, NULL);
 
        priv->max_conns = SOUP_SESSION_MAX_CONNS_DEFAULT;
        priv->max_conns_per_host = SOUP_SESSION_MAX_CONNS_PER_HOST_DEFAULT;
@@ -295,7 +292,6 @@ soup_session_dispose (GObject *object)
 
        priv->disposed = TRUE;
        soup_session_abort (session);
-       g_warn_if_fail (g_hash_table_size (priv->conns) == 0);
 
        while (priv->features)
                soup_session_remove_feature (session, priv->features->data);
@@ -315,7 +311,6 @@ soup_session_finalize (GObject *object)
        g_cond_clear (&priv->conn_cond);
        g_hash_table_destroy (priv->http_hosts);
        g_hash_table_destroy (priv->https_hosts);
-       g_hash_table_destroy (priv->conns);
 
        g_free (priv->user_agent);
        g_free (priv->accept_language);
@@ -1244,38 +1239,64 @@ soup_session_send_queue_item (SoupSession *session,
                soup_connection_send_request (item->conn, item, completion_cb, item);
 }
 
+/* Requires conn_lock to be locked */
+static GSList *
+get_all_connections (SoupSession *session)
+{
+       SoupSessionPrivate *priv = SOUP_SESSION_GET_PRIVATE (session);
+       GSList *conns = NULL, *host_conns;
+       GHashTableIter iter;
+       gpointer host;
+
+       g_hash_table_iter_init (&iter, priv->http_hosts);
+       while (g_hash_table_iter_next (&iter, NULL, &host)) {
+               host_conns = soup_session_host_get_connections (host);
+               conns = g_slist_concat (host_conns, conns);
+       }
+       g_hash_table_iter_init (&iter, priv->https_hosts);
+       while (g_hash_table_iter_next (&iter, NULL, &host)) {
+               host_conns = soup_session_host_get_connections (host);
+               conns = g_slist_concat (host_conns, conns);
+       }
+
+       return conns;
+}
+
 static gboolean
 soup_session_cleanup_connections (SoupSession *session,
                                  gboolean     cleanup_idle)
 {
        SoupSessionPrivate *priv = SOUP_SESSION_GET_PRIVATE (session);
-       GSList *conns = NULL, *c;
-       GHashTableIter iter;
-       gpointer conn, host;
+       GSList *conns, *disconnect_conns, *c;
+       SoupConnection *conn;
        SoupConnectionState state;
 
        g_mutex_lock (&priv->conn_lock);
-       g_hash_table_iter_init (&iter, priv->conns);
-       while (g_hash_table_iter_next (&iter, &conn, &host)) {
+
+       conns = get_all_connections (session);
+       disconnect_conns = NULL;
+       for (c = conns; c; c = c->next) {
+               conn = c->data;
                state = soup_connection_get_state (conn);
                if (state == SOUP_CONNECTION_REMOTE_DISCONNECTED ||
                    (cleanup_idle && state == SOUP_CONNECTION_IDLE)) {
-                       conns = g_slist_prepend (conns, g_object_ref (conn));
-                       g_hash_table_iter_remove (&iter);
-                       drop_connection (session, host, conn);
-               }
+                       disconnect_conns = g_slist_prepend (disconnect_conns, conn);
+                       drop_connection (session, conn);
+               } else
+                       g_object_unref (conn);
        }
+       g_slist_free (conns);
        g_mutex_unlock (&priv->conn_lock);
 
-       if (!conns)
+       if (!disconnect_conns)
                return FALSE;
 
-       for (c = conns; c; c = c->next) {
+       for (c = disconnect_conns; c; c = c->next) {
                conn = c->data;
                soup_connection_disconnect (conn);
                g_object_unref (conn);
        }
-       g_slist_free (conns);
+       g_slist_free (disconnect_conns);
 
        return TRUE;
 }
@@ -1308,18 +1329,12 @@ free_unused_host (SoupSessionHost *host,
        g_mutex_unlock (&priv->conn_lock);
 }
 
+/* Requires conn_lock to be locked */
 static void
-drop_connection (SoupSession *session, SoupSessionHost *host, SoupConnection *conn)
+drop_connection (SoupSession *session, SoupConnection *conn)
 {
        SoupSessionPrivate *priv = SOUP_SESSION_GET_PRIVATE (session);
 
-       /* Note: caller must hold conn_lock, and must remove @conn
-        * from priv->conns itself.
-        */
-
-       if (host)
-               soup_session_host_remove_connection (host, conn);
-
        g_signal_handlers_disconnect_by_func (conn, connection_disconnected, session);
        g_signal_handlers_disconnect_by_func (conn, connection_state_changed, session);
        priv->num_conns--;
@@ -1332,14 +1347,10 @@ connection_disconnected (SoupConnection *conn, gpointer user_data)
 {
        SoupSession *session = user_data;
        SoupSessionPrivate *priv = SOUP_SESSION_GET_PRIVATE (session);
-       SoupSessionHost *host;
 
        g_mutex_lock (&priv->conn_lock);
 
-       host = g_hash_table_lookup (priv->conns, conn);
-       if (host)
-               g_hash_table_remove (priv->conns, conn);
-       drop_connection (session, host, conn);
+       drop_connection (session, conn);
 
        g_mutex_unlock (&priv->conn_lock);
 
@@ -1703,8 +1714,6 @@ get_connection_for_host (SoupSession *session,
                 * conn_lock.
                 */
                g_signal_emit (session, signals[CONNECTION_CREATED], 0, conn);
-
-               g_hash_table_insert (priv->conns, conn, host);
                priv->num_conns++;
        }
 
@@ -2342,8 +2351,6 @@ soup_session_abort (SoupSession *session)
 {
        SoupSessionPrivate *priv;
        GSList *conns, *c;
-       GHashTableIter iter;
-       gpointer conn, host;
 
        g_return_if_fail (SOUP_IS_SESSION (session));
        priv = SOUP_SESSION_GET_PRIVATE (session);
@@ -2352,13 +2359,9 @@ soup_session_abort (SoupSession *session)
 
        /* Close all connections */
        g_mutex_lock (&priv->conn_lock);
-       conns = NULL;
-       g_hash_table_iter_init (&iter, priv->conns);
-       while (g_hash_table_iter_next (&iter, &conn, &host)) {
-               conns = g_slist_prepend (conns, g_object_ref (conn));
-               g_hash_table_iter_remove (&iter);
-               drop_connection (session, host, conn);
-       }
+       conns = get_all_connections (session);
+       for (c = conns; c; c = c->next)
+               drop_connection (session, c->data);
        g_mutex_unlock (&priv->conn_lock);
 
        for (c = conns; c; c = c->next) {