[evolution-data-server] Bug 628142 - Fix handling of simultaneous get_message requests
- From: David Woodhouse <dwmw2 src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [evolution-data-server] Bug 628142 - Fix handling of simultaneous get_message requests
- Date: Wed, 27 Apr 2011 23:47:25 +0000 (UTC)
commit bdd966164d997976a3f632fc14e1f7badb6c450b
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.
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 7a0e6ab..78c2c2c 100644
--- a/camel/providers/imapx/camel-imapx-server.c
+++ b/camel/providers/imapx/camel-imapx-server.c
@@ -4968,7 +4968,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);
@@ -5094,7 +5095,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 *
@@ -5193,20 +5195,36 @@ imapx_server_get_message (CamelIMAPXServer *is,
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);
@@ -5243,15 +5261,11 @@ imapx_server_get_message (CamelIMAPXServer *is,
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;
else if (job->u.get_message.stream != NULL)
@@ -5259,10 +5273,10 @@ imapx_server_get_message (CamelIMAPXServer *is,
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 6b46b18..67a1bb0 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]