[evolution-data-server/email-factory] Bug 628142 - Fix handling of simultaneous get_message requests



commit 4663e581586afaaa5c15d9b85ffaff65923dacbb
Author: David Woodhouse <David Woodhouse intel com>
Date:   Thu Apr 28 00:35:56 2011 +0100

    Bug 628142 - Fix handling of simultaneous get_message requests
    
    Drop the hash table of EFlags completely. It's broken, because the UID
    we use as the hash key isn't actually unique; the same UID can exist in
    multiple folders. And the lifetime issues on the EFlag weren't cleanly
    solvable (yeah, we can add a refcounting wrapper, but ick).
    
    We were *already* using imapx_is_job_in_queue() to check *properly* if
    there was an existing fetch. So just implement a simple 'fetch counter'
    with a GCond and a corresponding GMutex, bump that count by one each
    time any fetch completes, and use the GCond when waiting for a *specific*
    fetch to complete, inside a while (imapx_is_job_in_queue()) loop.
    (cherry picked from commit bdd966164d997976a3f632fc14e1f7badb6c450b)

 camel/providers/imapx/camel-imapx-server.c |   44 ++++++++++++++++++---------
 camel/providers/imapx/camel-imapx-server.h |    6 ++-
 2 files changed, 33 insertions(+), 17 deletions(-)
---
diff --git a/camel/providers/imapx/camel-imapx-server.c b/camel/providers/imapx/camel-imapx-server.c
index 1f4ebb4..10df89f 100644
--- a/camel/providers/imapx/camel-imapx-server.c
+++ b/camel/providers/imapx/camel-imapx-server.c
@@ -4807,7 +4807,8 @@ imapx_server_finalize (GObject *object)
 
 	g_static_rec_mutex_free(&is->queue_lock);
 	g_static_rec_mutex_free (&is->ostream_lock);
-	g_hash_table_destroy (is->uid_eflags);
+	g_mutex_free (is->fetch_mutex);
+	g_cond_free (is->fetch_cond);
 
 	camel_folder_change_info_free (is->changes);
 
@@ -4929,7 +4930,8 @@ camel_imapx_server_init (CamelIMAPXServer *is)
 	is->changes = camel_folder_change_info_new ();
 	is->parser_quit = FALSE;
 
-	is->uid_eflags = g_hash_table_new_full (g_str_hash, g_str_equal, (GDestroyNotify)g_free, (GDestroyNotify) e_flag_free);
+	is->fetch_mutex = g_mutex_new ();
+	is->fetch_cond = g_cond_new ();
 }
 
 CamelIMAPXServer *
@@ -5016,20 +5018,36 @@ imapx_server_get_message (CamelIMAPXServer *is, CamelFolder *folder, CamelOperat
 	CamelIMAPXJob *job;
 	CamelMessageInfo *mi;
 	gboolean registered;
-	EFlag *flag = NULL;
 	gboolean success;
 
 	QUEUE_LOCK (is);
 
 	if ((job = imapx_is_job_in_queue (is, folder, IMAPX_JOB_GET_MESSAGE, uid))) {
-		flag = g_hash_table_lookup (is->uid_eflags, uid);
-
 		if (pri > job->pri)
 			job->pri = pri;
 
-		QUEUE_UNLOCK (is);
+		/* Wait for the job to finish. This would be so much nicer if
+		   we could just use the queue lock with a GCond, but instead
+		   we have to use a GMutex. I miss the kernel waitqueues. */
+		do {
+			int this;
+
+			g_mutex_lock (is->fetch_mutex);
+			this = is->fetch_count;
+
+			QUEUE_UNLOCK (is);
+
+			while (is->fetch_count == this)
+				g_cond_wait (is->fetch_cond, is->fetch_mutex);
 
-		e_flag_wait (flag);
+			g_mutex_unlock (is->fetch_mutex);
+
+			QUEUE_LOCK (is);
+
+		} while (imapx_is_job_in_queue (is, folder,
+						IMAPX_JOB_GET_MESSAGE, uid));
+
+		QUEUE_UNLOCK (is);
 
 		stream = camel_data_cache_get (
 			ifolder->cache, "cur", uid, error);
@@ -5067,24 +5085,20 @@ imapx_server_get_message (CamelIMAPXServer *is, CamelFolder *folder, CamelOperat
 	job->u.get_message.size = ((CamelMessageInfoBase *) mi)->size;
 	camel_message_info_free (mi);
 	registered = imapx_register_job (is, job, error);
-	flag = e_flag_new ();
-	g_hash_table_insert (is->uid_eflags, g_strdup (uid), flag);
 
 	QUEUE_UNLOCK (is);
 
 	success = registered && imapx_run_job (is, job, error);
 
-	e_flag_set (flag);
-
 	if (success)
 		stream = job->u.get_message.stream;
 
 	g_free(job);
 
-	/* HACK FIXME just sleep for sometime so that the other waiting locks gets released by that time. Think of a
-	 better way..*/
-	g_usleep (1000);
-	g_hash_table_remove (is->uid_eflags, uid);
+	g_mutex_lock (is->fetch_mutex);
+	is->fetch_count++;
+	g_cond_broadcast (is->fetch_cond);
+	g_mutex_unlock (is->fetch_mutex);
 
 	return stream;
 }
diff --git a/camel/providers/imapx/camel-imapx-server.h b/camel/providers/imapx/camel-imapx-server.h
index 1f9e884..1050fec 100644
--- a/camel/providers/imapx/camel-imapx-server.h
+++ b/camel/providers/imapx/camel-imapx-server.h
@@ -119,8 +119,10 @@ struct _CamelIMAPXServer {
 
 	gboolean use_qresync;
 
-	/* used for storing eflags to syncronize duplicate get_message requests */
-	GHashTable *uid_eflags;
+	/* used to synchronize duplicate get_message requests */
+	GCond *fetch_cond;
+	GMutex *fetch_mutex;
+	int fetch_count;
 };
 
 struct _CamelIMAPXServerClass {



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]