diff options
author | David Woodhouse <David.Woodhouse@intel.com> | 2011-04-28 00:35:56 +0100 |
---|---|---|
committer | David Woodhouse <David.Woodhouse@intel.com> | 2011-04-28 00:44:00 +0100 |
commit | e37ca87c8e24e3bf8e9a481d0a444026810a7439 (patch) | |
tree | eb1338c4d9eb1bd147ff3f0afc63b7bfb2b6be74 | |
parent | 5be5b05f6452140ee59f5858d5e441919f7afb9e (diff) | |
download | evolution-data-server-e37ca87c8e24e3bf8e9a481d0a444026810a7439.tar.gz |
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)
-rw-r--r-- | camel/providers/imapx/camel-imapx-server.c | 44 | ||||
-rw-r--r-- | 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 179252d08..f54e6a843 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 1f9e88418..1050fece0 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 { |