Bug 750766 - [IMAPx] Deadlock filtering and downloading messages for offline use

5 files changed, 87 insertions, 74 deletions

diff --git a/camel/providers/imapx/camel-imapx-job.c b/camel/providers/imapx/camel-imapx-job.c
index c470754b6..b1f949cde 100644
--- a/camel/providers/imapx/camel-imapx-job.c
+++ b/camel/providers/imapx/camel-imapx-job.c
@@ -48,6 +48,7 @@ struct _CamelIMAPXRealJob {
 	GMutex mailbox_lock;
 
 	CamelIMAPXMailbox *guard_mailbox_update; /* uses the mailbox_lock */
+	gint has_update_locked;
 };
 
 static void
@@ -127,10 +128,11 @@ camel_imapx_job_unref (CamelIMAPXJob *job)
 			real_job->destroy_data (real_job->data);
 
 		g_mutex_lock (&real_job->mailbox_lock);
-		if (real_job->guard_mailbox_update) {
-			camel_imapx_mailbox_unlock_update (real_job->guard_mailbox_update);
-			g_clear_object (&real_job->guard_mailbox_update);
+		while (real_job->has_update_locked > 0) {
+			camel_imapx_mailbox_inc_update_count (real_job->guard_mailbox_update, -1);
+			real_job->has_update_locked--;
 		}
+		g_clear_object (&real_job->guard_mailbox_update);
 		g_mutex_unlock (&real_job->mailbox_lock);
 
 		g_clear_object (&real_job->mailbox);
@@ -242,10 +244,11 @@ camel_imapx_job_done (CamelIMAPXJob *job)
 	real_job = (CamelIMAPXRealJob *) job;
 
 	g_mutex_lock (&real_job->mailbox_lock);
-	if (real_job->guard_mailbox_update) {
-		camel_imapx_mailbox_unlock_update (real_job->guard_mailbox_update);
-		g_clear_object (&real_job->guard_mailbox_update);
+	while (real_job->has_update_locked > 0) {
+		camel_imapx_mailbox_inc_update_count (real_job->guard_mailbox_update, -1);
+		real_job->has_update_locked--;
 	}
+	g_clear_object (&real_job->guard_mailbox_update);
 	g_mutex_unlock (&real_job->mailbox_lock);
 
 	g_mutex_lock (&real_job->done_mutex);
@@ -279,36 +282,6 @@ camel_imapx_job_run (CamelIMAPXJob *job,
 	return success;
 }
 
-void
-camel_imapx_job_guard_mailbox_update (CamelIMAPXJob *job,
-				      CamelIMAPXMailbox *mailbox)
-{
-	CamelIMAPXRealJob *real_job;
-
-	g_return_if_fail (CAMEL_IS_IMAPX_JOB (job));
-
-	if (mailbox)
-		g_return_if_fail (CAMEL_IS_IMAPX_MAILBOX (mailbox));
-
-	real_job = (CamelIMAPXRealJob *) job;
-
-	g_mutex_lock (&real_job->mailbox_lock);
-
-	if (mailbox != real_job->guard_mailbox_update) {
-		if (real_job->guard_mailbox_update) {
-			camel_imapx_mailbox_unlock_update (real_job->guard_mailbox_update);
-			g_clear_object (&real_job->guard_mailbox_update);
-		}
-
-		if (mailbox) {
-			real_job->guard_mailbox_update = g_object_ref (mailbox);
-			camel_imapx_mailbox_lock_update (real_job->guard_mailbox_update);
-		}
-	}
-
-	g_mutex_unlock (&real_job->mailbox_lock);
-}
-
 gboolean
 camel_imapx_job_matches (CamelIMAPXJob *job,
                          CamelIMAPXMailbox *mailbox,
@@ -493,3 +466,23 @@ camel_imapx_job_set_error_if_failed (CamelIMAPXJob *job,
 
 	return g_cancellable_set_error_if_cancelled (real_job->cancellable, error);
 }
+
+void
+camel_imapx_job_inc_update_locked (CamelIMAPXJob *job,
+				   CamelIMAPXMailbox *mailbox)
+{
+	CamelIMAPXRealJob *real_job;
+
+	g_return_val_if_fail (CAMEL_IS_IMAPX_JOB (job), TRUE);
+
+	real_job = (CamelIMAPXRealJob *) job;
+
+	g_mutex_lock (&real_job->mailbox_lock);
+	if (real_job->guard_mailbox_update) {
+		g_warn_if_fail (real_job->guard_mailbox_update == mailbox);
+	} else {
+		real_job->guard_mailbox_update = g_object_ref (mailbox);
+	}
+	real_job->has_update_locked++;
+	g_mutex_unlock (&real_job->mailbox_lock);
+}
diff --git a/camel/providers/imapx/camel-imapx-job.h b/camel/providers/imapx/camel-imapx-job.h
index cf09fc33f..b82e36a05 100644
--- a/camel/providers/imapx/camel-imapx-job.h
+++ b/camel/providers/imapx/camel-imapx-job.h
@@ -64,9 +64,6 @@ void		camel_imapx_job_done		(CamelIMAPXJob *job);
 gboolean	camel_imapx_job_run		(CamelIMAPXJob *job,
 						 CamelIMAPXServer *is,
 						 GError **error);
-void		camel_imapx_job_guard_mailbox_update
-						(CamelIMAPXJob *job,
-						 CamelIMAPXMailbox *mailbox);
 gboolean	camel_imapx_job_matches		(CamelIMAPXJob *job,
 						 CamelIMAPXMailbox *mailbox,
 						 const gchar *uid);
@@ -86,6 +83,9 @@ void		camel_imapx_job_take_error	(CamelIMAPXJob *job,
 gboolean	camel_imapx_job_set_error_if_failed
 						(CamelIMAPXJob *job,
 						 GError **error);
+void		camel_imapx_job_inc_update_locked
+						(CamelIMAPXJob *job,
+						 CamelIMAPXMailbox *mailbox);
 
 G_END_DECLS
 
diff --git a/camel/providers/imapx/camel-imapx-mailbox.c b/camel/providers/imapx/camel-imapx-mailbox.c
index 5e576e4d6..501dd5677 100644
--- a/camel/providers/imapx/camel-imapx-mailbox.c
+++ b/camel/providers/imapx/camel-imapx-mailbox.c
@@ -52,8 +52,7 @@ struct _CamelIMAPXMailboxPrivate {
 
 	GMutex property_lock;
 	GMutex update_lock;
-	GCond update_cond;
-	gboolean update_is_locked;
+	gint update_count;
 
 	/* Protected by the "property_lock". */
 	GHashTable *attributes;
@@ -101,7 +100,6 @@ imapx_mailbox_finalize (GObject *object)
 
 	g_mutex_clear (&priv->property_lock);
 	g_mutex_clear (&priv->update_lock);
-	g_cond_clear (&priv->update_cond);
 	g_hash_table_destroy (priv->attributes);
 	g_sequence_free (priv->message_map);
 	g_strfreev (priv->quota_roots);
@@ -129,11 +127,10 @@ camel_imapx_mailbox_init (CamelIMAPXMailbox *mailbox)
 
 	g_mutex_init (&mailbox->priv->property_lock);
 	g_mutex_init (&mailbox->priv->update_lock);
-	g_cond_init (&mailbox->priv->update_cond);
-	mailbox->priv->update_is_locked = FALSE;
 	mailbox->priv->message_map = g_sequence_new (NULL);
 	mailbox->priv->permanentflags = ~0;
 	mailbox->priv->state = CAMEL_IMAPX_MAILBOX_STATE_CREATED;
+	mailbox->priv->update_count = 0;
 }
 
 /**
@@ -1197,35 +1194,23 @@ camel_imapx_mailbox_handle_status_response (CamelIMAPXMailbox *mailbox,
 		mailbox->priv->highestmodseq = value64;
 }
 
-/* Prevents running FETCH and STORE at the same time for the given mailbox */
-void
-camel_imapx_mailbox_lock_update (CamelIMAPXMailbox *mailbox)
+gint
+camel_imapx_mailbox_get_update_count (CamelIMAPXMailbox *mailbox)
 {
-	g_return_if_fail (CAMEL_IS_IMAPX_MAILBOX (mailbox));
+	gint res;
 
 	g_mutex_lock (&mailbox->priv->update_lock);
-
-	while (mailbox->priv->update_is_locked) {
-		g_cond_wait (&mailbox->priv->update_cond, &mailbox->priv->update_lock);
-	}
-
-	mailbox->priv->update_is_locked = TRUE;
-
+	res = mailbox->priv->update_count;
 	g_mutex_unlock (&mailbox->priv->update_lock);
+
+	return res;
 }
 
-/* Prevents running FETCH and STORE at the same time for the given mailbox */
 void
-camel_imapx_mailbox_unlock_update (CamelIMAPXMailbox *mailbox)
+camel_imapx_mailbox_inc_update_count (CamelIMAPXMailbox *mailbox,
+				      gint inc)
 {
-	g_return_if_fail (CAMEL_IS_IMAPX_MAILBOX (mailbox));
-
 	g_mutex_lock (&mailbox->priv->update_lock);
-
-	if (mailbox->priv->update_is_locked) {
-		mailbox->priv->update_is_locked = FALSE;
-		g_cond_signal (&mailbox->priv->update_cond);
-	}
-
+	mailbox->priv->update_count += inc;
 	g_mutex_unlock (&mailbox->priv->update_lock);
 }
diff --git a/camel/providers/imapx/camel-imapx-mailbox.h b/camel/providers/imapx/camel-imapx-mailbox.h
index b8de9a9ea..139b7ee86 100644
--- a/camel/providers/imapx/camel-imapx-mailbox.h
+++ b/camel/providers/imapx/camel-imapx-mailbox.h
@@ -175,10 +175,11 @@ void		camel_imapx_mailbox_handle_status_response
 					(CamelIMAPXMailbox *mailbox,
 					 CamelIMAPXStatusResponse *response);
 
-void		camel_imapx_mailbox_lock_update
-					(CamelIMAPXMailbox *mailbox);
-void		camel_imapx_mailbox_unlock_update
+gint		camel_imapx_mailbox_get_update_count
 					(CamelIMAPXMailbox *mailbox);
+void		camel_imapx_mailbox_inc_update_count
+					(CamelIMAPXMailbox *mailbox,
+					 gint inc);
 
 G_END_DECLS
 
diff --git a/camel/providers/imapx/camel-imapx-server.c b/camel/providers/imapx/camel-imapx-server.c
index 165a07f7e..b14c1d59c 100644
--- a/camel/providers/imapx/camel-imapx-server.c
+++ b/camel/providers/imapx/camel-imapx-server.c
@@ -1294,8 +1294,11 @@ imapx_command_start (CamelIMAPXServer *is,
 		camel_imapx_command_queue_remove (is->active, ic);
 		imapx_server_command_removed (is, ic);
 
-		if (ic->complete != NULL)
+		if (ic->complete != NULL) {
+			QUEUE_UNLOCK (is);
 			ic->complete (is, ic);
+			QUEUE_LOCK (is);
+		}
 
 		if (is->literal == ic)
 			is->literal = NULL;
@@ -1361,8 +1364,11 @@ fail:
 	/* Hand the error off to the command that we failed to start. */
 	camel_imapx_command_failed (ic, local_error);
 
-	if (ic->complete != NULL)
+	if (ic->complete != NULL) {
+		QUEUE_UNLOCK (is);
 		ic->complete (is, ic);
+		QUEUE_LOCK (is);
+	}
 
 	g_clear_error (&local_error);
 
@@ -8906,6 +8912,17 @@ camel_imapx_server_refresh_info (CamelIMAPXServer *is,
 	if (!imapx_ensure_mailbox_permanentflags (is, mailbox, cancellable, error))
 		return NULL;
 
+	/* Wait for any SyncChanges jobs to finish before running the refresh */
+	while (job = imapx_server_ref_job (is, mailbox, IMAPX_JOB_SYNC_CHANGES, NULL), job != NULL) {
+		/* Promote the existing job's priority if ours is higher. */
+		if (IMAPX_PRIORITY_REFRESH_INFO > job->pri)
+			job->pri = IMAPX_PRIORITY_REFRESH_INFO;
+
+		/* Wait for the job to finish. */
+		camel_imapx_job_wait (job, NULL);
+		camel_imapx_job_unref (job);
+	}
+
 	QUEUE_LOCK (is);
 
 	data = g_slice_new0 (RefreshInfoData);
@@ -8931,8 +8948,10 @@ camel_imapx_server_refresh_info (CamelIMAPXServer *is,
 
 	QUEUE_UNLOCK (is);
 
-	if (registered)
-		camel_imapx_job_guard_mailbox_update (job, mailbox);
+	if (registered) {
+		camel_imapx_mailbox_inc_update_count (mailbox, 1);
+		camel_imapx_job_inc_update_locked (job, mailbox);
+	}
 
 	if (registered && camel_imapx_job_run (job, is, error)) {
 		changes = data->changes;
@@ -9241,6 +9260,19 @@ imapx_server_sync_changes (CamelIMAPXServer *is,
 		own_allocated_changed_uids = TRUE;
 	}
 
+	if (job_type == IMAPX_JOB_SYNC_CHANGES) {
+		/* Wait for any RefreshInfo jobs to finish before running the sync */
+		while (job = imapx_server_ref_job (is, mailbox, IMAPX_JOB_REFRESH_INFO, NULL), job != NULL) {
+			/* Promote the existing job's priority if ours is higher. */
+			if (pri > job->pri)
+				job->pri = pri;
+
+			/* Wait for the job to finish. */
+			camel_imapx_job_wait (job, NULL);
+			camel_imapx_job_unref (job);
+		}
+	}
+
 	QUEUE_LOCK (is);
 
 	data = g_slice_new0 (SyncChangesData);
@@ -9268,8 +9300,10 @@ imapx_server_sync_changes (CamelIMAPXServer *is,
 
 	QUEUE_UNLOCK (is);
 
-	if (job_type == IMAPX_JOB_SYNC_CHANGES && registered)
-		camel_imapx_job_guard_mailbox_update (job, mailbox);
+	if (job_type == IMAPX_JOB_SYNC_CHANGES && registered) {
+		camel_imapx_mailbox_inc_update_count (mailbox, 1);
+		camel_imapx_job_inc_update_locked (job, mailbox);
+	}
 
 	success = registered && camel_imapx_job_run (job, is, error);