CamelIMAPXServer: Rewrite duplicate message fetching.

The g_cond_wait() call in imapx_server_get_message() seems to be the
source of many deadlocks.  Possibly because some of the error handling
branches in that function fail to broadcast the GCond being waited on,
and cancellation is a very common error condition, especially when the
user is rapidly browsing through messages.

The code is attempting to wait for the completion of a GET_MESSAGE job
for the same UID which is already queued.  It's doing this by defining
its own GMutex and GCond, but it turns out we already have a function
for this: camel_imapx_job_wait().

Let's try using camel_imapx_job_wait() there.  It can't do worse than
the current situation, and we can kill off the specialized GMutex and
GCond variables which were unfortunately made public.

diff --git a/camel/camel-imapx-server.c b/camel/camel-imapx-server.c
index 0542049..1bd5565 100644 (file)
--- a/camel/camel-imapx-server.c
+++ b/camel/camel-imapx-server.c
@@ -7166,8 +7166,6 @@ imapx_server_finalize (GObject *object)
 
        g_rec_mutex_clear (&is->queue_lock);
        g_mutex_clear (&is->select_lock);
-       g_mutex_clear (&is->fetch_mutex);
-       g_cond_clear (&is->fetch_cond);
 
        camel_folder_change_info_free (is->changes);
 
@@ -7297,9 +7295,6 @@ camel_imapx_server_init (CamelIMAPXServer *is)
        is->changes = camel_folder_change_info_new ();
        is->parser_quit = FALSE;
 
-       g_mutex_init (&is->fetch_mutex);
-       g_cond_init (&is->fetch_cond);
-
        is->priv->known_alerts = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
 }
 
@@ -7430,32 +7425,16 @@ imapx_server_get_message (CamelIMAPXServer *is,
        QUEUE_LOCK (is);
 
        if ((job = imapx_is_job_in_queue (is, folder, IMAPX_JOB_GET_MESSAGE, uid))) {
+               /* Promote the existing GET_MESSAGE
+                * job's priority if ours is higher. */
                if (pri > job->pri)
                        job->pri = pri;
 
-               /* 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 {
-                       gint 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);
-
-                       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);
 
+               /* Wait for the job to finish. */
+               camel_imapx_job_wait (job);
+
                stream = camel_data_cache_get (
                        ifolder->cache, "cur", uid, error);
                if (stream == NULL)
@@ -7505,11 +7484,6 @@ imapx_server_get_message (CamelIMAPXServer *is,
 
        camel_imapx_job_unref (job);
 
-       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/camel-imapx-server.h b/camel/camel-imapx-server.h
index b23447e..f931265 100644 (file)
--- a/camel/camel-imapx-server.h
+++ b/camel/camel-imapx-server.h
@@ -159,11 +159,6 @@ struct _CamelIMAPXServer {
        gboolean use_idle;
 
        gboolean use_qresync;
-
-       /* used to synchronize duplicate get_message requests */
-       GCond fetch_cond;
-       GMutex fetch_mutex;
-       gint fetch_count;
 };
 
 struct _CamelIMAPXServerClass {