[evolution-data-server] CamelIMAPXConnManager: Similarly for connection list operations.

[Matthew Barnes <mbarnes src gnome org>]

commit c520cf428aeff864d1d0244b4589242e50915fbf
Author: Matthew Barnes <mbarnes redhat com>
Date:   Mon Jan 23 13:11:13 2012 -0500

    CamelIMAPXConnManager: Similarly for connection list operations.
    
    Define a bunch of simple thread-safe operations for the list:
    
       imapx_conn_manager_list_info
       imapx_conn_manager_lookup_info
       imapx_conn_manager_remove_info
    
    * Note: lookup_info() returns a new ConnectionInfo reference, and
            list_info() returns a new GList of ConnectionInfo structs,
            each with a new reference
    
    Also rewrite imapx_find_connection() to make the logic clearer.
    (Had to stare at that function for a long time to understand it.)

 camel/providers/imapx/camel-imapx-conn-manager.c |  263 ++++++++++++++--------
 1 files changed, 165 insertions(+), 98 deletions(-)
---
diff --git a/camel/providers/imapx/camel-imapx-conn-manager.c b/camel/providers/imapx/camel-imapx-conn-manager.c
index c80d796..e65e880 100644
--- a/camel/providers/imapx/camel-imapx-conn-manager.c
+++ b/camel/providers/imapx/camel-imapx-conn-manager.c
@@ -208,6 +208,79 @@ connection_info_set_selected_folder (ConnectionInfo *cinfo,
 	g_mutex_unlock (cinfo->lock);
 }
 
+static GList *
+imapx_conn_manager_list_info (CamelIMAPXConnManager *con_man)
+{
+	GList *list;
+
+	g_return_val_if_fail (CAMEL_IS_IMAPX_CONN_MANAGER (con_man), NULL);
+
+	CON_LOCK (con_man);
+
+	list = g_list_copy (con_man->priv->connections);
+	g_list_foreach (list, (GFunc) connection_info_ref, NULL);
+
+	CON_UNLOCK (con_man);
+
+	return list;
+}
+
+static ConnectionInfo *
+imapx_conn_manager_lookup_info (CamelIMAPXConnManager *con_man,
+                                CamelIMAPXServer *is)
+{
+	ConnectionInfo *cinfo = NULL;
+	GList *list, *link;
+
+	g_return_val_if_fail (CAMEL_IS_IMAPX_CONN_MANAGER (con_man), NULL);
+	g_return_val_if_fail (CAMEL_IS_IMAPX_SERVER (is), NULL);
+
+	CON_LOCK (con_man);
+
+	list = con_man->priv->connections;
+
+	for (link = list; link != NULL; link = g_list_next (link)) {
+		ConnectionInfo *candidate = link->data;
+
+		if (candidate->is == is) {
+			cinfo = connection_info_ref (candidate);
+			break;
+		}
+	}
+
+	CON_UNLOCK (con_man);
+
+	return cinfo;
+}
+
+static gboolean
+imapx_conn_manager_remove_info (CamelIMAPXConnManager *con_man,
+                                ConnectionInfo *cinfo)
+{
+	GList *list, *link;
+	gboolean removed = FALSE;
+
+	g_return_val_if_fail (CAMEL_IS_IMAPX_CONN_MANAGER (con_man), FALSE);
+	g_return_val_if_fail (cinfo != NULL, FALSE);
+
+	CON_LOCK (con_man);
+
+	list = con_man->priv->connections;
+	link = g_list_find (list, cinfo);
+
+	if (link != NULL) {
+		list = g_list_remove_link (list, link);
+		connection_info_unref (cinfo);
+		removed = TRUE;
+	}
+
+	con_man->priv->connections = list;
+
+	CON_UNLOCK (con_man);
+
+	return removed;
+}
+
 static void
 imapx_prune_connections (CamelIMAPXConnManager *con_man)
 {
@@ -335,8 +408,6 @@ camel_imapx_conn_manager_init (CamelIMAPXConnManager *con_man)
 	con_man->priv = CAMEL_IMAPX_CONN_MANAGER_GET_PRIVATE (con_man);
 
 	g_static_rec_mutex_init (&con_man->priv->con_man_lock);
-
-	con_man->priv->clearing_connections = FALSE;
 }
 
 /* Static functions go here */
@@ -346,9 +417,7 @@ static void
 imapx_conn_shutdown (CamelIMAPXServer *is,
                      CamelIMAPXConnManager *con_man)
 {
-	GList *l;
 	ConnectionInfo *cinfo;
-	gboolean found = FALSE;
 
 	/* when clearing connections then other thread than a parser thread,
 	 * in which this function is called, holds the CON_LOCK, thus skip
@@ -359,22 +428,13 @@ imapx_conn_shutdown (CamelIMAPXServer *is,
 		return;
 	}
 
-	CON_LOCK (con_man);
+	/* Returns a new ConnectionInfo reference. */
+	cinfo = imapx_conn_manager_lookup_info (con_man, is);
 
-	for (l = con_man->priv->connections; l != NULL; l = g_list_next (l)) {
-		cinfo = (ConnectionInfo *) l->data;
-		if (cinfo->is == is) {
-			found = TRUE;
-			break;
-		}
-	}
-
-	if (found) {
-		con_man->priv->connections = g_list_remove (con_man->priv->connections, cinfo);
+	if (cinfo != NULL) {
+		imapx_conn_manager_remove_info (con_man, cinfo);
 		connection_info_unref (cinfo);
 	}
-
-	CON_UNLOCK (con_man);
 }
 
 static void
@@ -382,42 +442,33 @@ imapx_conn_update_select (CamelIMAPXServer *is,
                           const gchar *selected_folder,
                           CamelIMAPXConnManager *con_man)
 {
-	GList *l;
 	ConnectionInfo *cinfo;
-	gboolean found = FALSE;
+	gchar *old_selected_folder;
 
-	CON_LOCK (con_man);
+	/* Returns a new ConnectionInfo reference. */
+	cinfo = imapx_conn_manager_lookup_info (con_man, is);
 
-	for (l = con_man->priv->connections; l != NULL; l = g_list_next (l)) {
-		cinfo = (ConnectionInfo *) l->data;
-		if (cinfo->is == is) {
-			found = TRUE;
-			break;
-		}
-	}
-
-	if (found) {
-		gchar *old_selected_folder;
+	if (cinfo == NULL)
+		return;
 
-		old_selected_folder =
-			connection_info_dup_selected_folder (cinfo);
+	old_selected_folder = connection_info_dup_selected_folder (cinfo);
 
-		if (old_selected_folder != NULL) {
-			IMAPXJobQueueInfo *jinfo;
+	if (old_selected_folder != NULL) {
+		IMAPXJobQueueInfo *jinfo;
 
-			jinfo = camel_imapx_server_get_job_queue_info (cinfo->is);
-			if (!g_hash_table_lookup (jinfo->folders, old_selected_folder)) {
-				connection_info_remove_folder_name (cinfo, old_selected_folder);
-				c(is->tagprefix, "Removed folder %s from connection folder list - select changed \n", old_selected_folder);
-			}
-			camel_imapx_destroy_job_queue_info (jinfo);
-			g_free (old_selected_folder);
+		jinfo = camel_imapx_server_get_job_queue_info (is);
+		if (!g_hash_table_lookup (jinfo->folders, old_selected_folder)) {
+			connection_info_remove_folder_name (cinfo, old_selected_folder);
+			c(is->tagprefix, "Removed folder %s from connection folder list - select changed \n", old_selected_folder);
 		}
+		camel_imapx_destroy_job_queue_info (jinfo);
 
-		connection_info_set_selected_folder (cinfo, selected_folder);
+		g_free (old_selected_folder);
 	}
 
-	CON_UNLOCK (con_man);
+	connection_info_set_selected_folder (cinfo, selected_folder);
+
+	connection_info_unref (cinfo);
 }
 
 /* This should find a connection if the slots are full, returns NULL if there are slots available for a new connection for a folder */
@@ -425,13 +476,13 @@ static CamelIMAPXServer *
 imapx_find_connection (CamelIMAPXConnManager *con_man,
                        const gchar *folder_name)
 {
-	guint i = 0, prev_len = -1, n = -1;
-	GList *l;
-	CamelIMAPXServer *is = NULL;
 	CamelService *service;
 	CamelSettings *settings;
+	CamelIMAPXServer *is = NULL;
 	ConnectionInfo *cinfo = NULL;
+	GList *list, *link;
 	guint concurrent_connections;
+	guint min_jobs = G_MAXUINT;
 
 	CON_LOCK (con_man);
 
@@ -442,40 +493,62 @@ imapx_find_connection (CamelIMAPXConnManager *con_man,
 		camel_imapx_settings_get_concurrent_connections (
 		CAMEL_IMAPX_SETTINGS (settings));
 
-	/* Have a dedicated connection for INBOX ? */
-	for (l = con_man->priv->connections, i = 0; l != NULL; l = g_list_next (l), i++) {
-		IMAPXJobQueueInfo *jinfo = NULL;
+	/* XXX Have a dedicated connection for INBOX ? */
 
-		cinfo = (ConnectionInfo *) l->data;
-		jinfo = camel_imapx_server_get_job_queue_info (cinfo->is);
+	list = con_man->priv->connections;
 
-		if (prev_len == -1) {
-			prev_len = jinfo->queue_len;
-			n = 0;
-		}
+	/* If a folder was not given, find the least-busy connection. */
+	if (folder_name == NULL)
+		goto least_busy;
 
-		if (jinfo->queue_len < prev_len)
-			n = i;
+	/* First try to find a connection already handling this folder. */
+	for (link = list; link != NULL; link = g_list_next (link)) {
+		ConnectionInfo *candidate = link->data;
 
-		camel_imapx_destroy_job_queue_info (jinfo);
+		if (connection_info_has_folder_name (candidate, folder_name)) {
+			cinfo = connection_info_ref (candidate);
+			goto exit;
+		}
+	}
 
-		if (folder_name != NULL && (connection_info_has_folder_name (cinfo, folder_name) || connection_info_is_available (cinfo))) {
-			is = g_object_ref (cinfo->is);
+	/* Next try to find a connection not handling any folders. */
+	for (link = list; link != NULL; link = g_list_next (link)) {
+		ConnectionInfo *candidate = link->data;
 
-			connection_info_insert_folder_name (cinfo, folder_name);
-			c(is->tagprefix, "Found connection for %s and connection number %d \n", folder_name, i+1);
-			break;
+		if (connection_info_is_available (candidate)) {
+			cinfo = connection_info_ref (candidate);
+			goto exit;
 		}
 	}
 
-	if (is == NULL && n != -1 && (!folder_name || concurrent_connections == g_list_length (con_man->priv->connections))) {
-		cinfo = g_list_nth_data (con_man->priv->connections, n);
-		is = g_object_ref (cinfo->is);
+least_busy:
+	/* Pick the connection with the least number of jobs in progress. */
+	for (link = list; link != NULL; link = g_list_next (link)) {
+		ConnectionInfo *candidate = link->data;
+		IMAPXJobQueueInfo *jinfo = NULL;
 
-		if (folder_name != NULL) {
-			connection_info_insert_folder_name (cinfo, folder_name);
-			c(is->tagprefix, "Adding folder %s to connection number %d \n", folder_name, n+1);
+		jinfo = camel_imapx_server_get_job_queue_info (candidate->is);
+
+		if (cinfo == NULL) {
+			cinfo = connection_info_ref (candidate);
+			min_jobs = jinfo->queue_len;
+
+		} else if (jinfo->queue_len < min_jobs) {
+			connection_info_unref (cinfo);
+			cinfo = connection_info_ref (candidate);
+			min_jobs = jinfo->queue_len;
 		}
+
+		camel_imapx_destroy_job_queue_info (jinfo);
+	}
+
+exit:
+	if (cinfo != NULL && folder_name != NULL)
+		connection_info_insert_folder_name (cinfo, folder_name);
+
+	if (cinfo != NULL) {
+		is = g_object_ref (cinfo->is);
+		connection_info_unref (cinfo);
 	}
 
 	if (camel_debug_flag (conman))
@@ -553,7 +626,9 @@ imapx_create_new_connection (CamelIMAPXConnManager *con_man,
 	if (folder_name != NULL)
 		connection_info_insert_folder_name (cinfo, folder_name);
 
-	con_man->priv->connections = g_list_prepend (con_man->priv->connections, cinfo);
+	/* Takes ownership of the ConnectionInfo. */
+	con_man->priv->connections = g_list_prepend (
+		con_man->priv->connections, cinfo);
 
 	c(is->tagprefix, "Created new connection for %s and total connections %d \n", folder_name, g_list_length (con_man->priv->connections));
 
@@ -605,19 +680,20 @@ camel_imapx_conn_manager_get_connection (CamelIMAPXConnManager *con_man,
 GList *
 camel_imapx_conn_manager_get_connections (CamelIMAPXConnManager *con_man)
 {
-	GList *l, *conns = NULL;
+	GList *list, *link;
 
-	CON_LOCK (con_man);
+	g_return_val_if_fail (CAMEL_IS_IMAPX_CONN_MANAGER (con_man), NULL);
 
-	for (l = con_man->priv->connections; l != NULL; l = g_list_next (l)) {
-		ConnectionInfo *cinfo = (ConnectionInfo *) l->data;
+	list = imapx_conn_manager_list_info (con_man);
 
-		conns = g_list_prepend (conns, g_object_ref (cinfo->is));
+	/* Swap ConnectionInfo for CamelIMAPXServer in each link. */
+	for (link = list; link != NULL; link = g_list_next (link)) {
+		ConnectionInfo *cinfo = link->data;
+		link->data = g_object_ref (cinfo->is);
+		connection_info_unref (cinfo);
 	}
 
-	CON_UNLOCK (con_man);
-
-	return conns;
+	return list;
 }
 
 /* Used for handling operations that fails to execute and that needs to removed from folder list */
@@ -626,34 +702,25 @@ camel_imapx_conn_manager_update_con_info (CamelIMAPXConnManager *con_man,
                                           CamelIMAPXServer *is,
                                           const gchar *folder_name)
 {
-	GList *l;
 	ConnectionInfo *cinfo;
-	gboolean found = FALSE;
+	IMAPXJobQueueInfo *jinfo;
 
 	g_return_if_fail (CAMEL_IS_IMAPX_CONN_MANAGER (con_man));
 
-	CON_LOCK (con_man);
-
-	for (l = con_man->priv->connections; l != NULL; l = g_list_next (l)) {
-		cinfo = (ConnectionInfo *) l->data;
-		if (cinfo->is == is) {
-			found = TRUE;
-			break;
-		}
-	}
+	/* Returns a new ConnectionInfo reference. */
+	cinfo = imapx_conn_manager_lookup_info (con_man, is);
 
-	if (found) {
-		IMAPXJobQueueInfo *jinfo;
+	if (cinfo == NULL)
+		return;
 
-		jinfo = camel_imapx_server_get_job_queue_info (cinfo->is);
-		if (!g_hash_table_lookup (jinfo->folders, folder_name)) {
-			connection_info_remove_folder_name (cinfo, folder_name);
-			c(is->tagprefix, "Removed folder %s from connection folder list - op done \n", folder_name);
-		}
-		camel_imapx_destroy_job_queue_info (jinfo);
+	jinfo = camel_imapx_server_get_job_queue_info (cinfo->is);
+	if (!g_hash_table_lookup (jinfo->folders, folder_name)) {
+		connection_info_remove_folder_name (cinfo, folder_name);
+		c(is->tagprefix, "Removed folder %s from connection folder list - op done \n", folder_name);
 	}
+	camel_imapx_destroy_job_queue_info (jinfo);
 
-	CON_UNLOCK (con_man);
+	connection_info_unref (cinfo);
 }
 
 void