Bug #666327 - IMAP deadlock when moving message and checking new mail

with service lock removal the fix for bug #666327
went out (commit 74fcab535c0f50a27742c05e94036b8370ea9173).
Was a good thing as this version is less a hack.
To summarize:
imap folder deadlock:
1. do_copy :
	.imap command on source lock the imap store
2. camel_imap_folder_fetch_data on destination:
	. lock the destination folder
3. do_copy :
	. call the hande user tag : wait for lock on the destination
folder cache	.
4. camel_imap_folder_fetch_data on destination:
	. wait for lock on the imap store to be freed.

Fix: get the lock on the destination folder cache before locking the
imap store (ie before calling the imap command).

diff --git a/camel/providers/imap/camel-imap-folder.c b/camel/providers/imap/camel-imap-folder.c
index 9570fd7..ee9a69f 100644 (file)
--- a/camel/providers/imap/camel-imap-folder.c
+++ b/camel/providers/imap/camel-imap-folder.c
@@ -2609,6 +2609,7 @@ imap_transfer_offline (CamelFolder *source,
        return TRUE;
 }
 
+/* Call with lock held on destination folder cache */
 static void
 handle_copyuid (CamelImapResponse *response,
                 CamelFolder *source,
@@ -2644,7 +2645,6 @@ handle_copyuid (CamelImapResponse *response,
                 * command lock too, so no one else could be here.
                 */
                CAMEL_IMAP_FOLDER_REC_LOCK (source, cache_lock);
-               CAMEL_IMAP_FOLDER_REC_LOCK (destination, cache_lock);
                for (i = 0; i < src->len; i++) {
                        camel_imap_message_cache_copy (scache, src->pdata[i],
                                                       dcache, dest->pdata[i]);
@@ -2652,7 +2652,6 @@ handle_copyuid (CamelImapResponse *response,
                        imap_folder_add_ignore_recent (CAMEL_IMAP_FOLDER (destination), dest->pdata[i]);
                }
                CAMEL_IMAP_FOLDER_REC_UNLOCK (source, cache_lock);
-               CAMEL_IMAP_FOLDER_REC_UNLOCK (destination, cache_lock);
 
                imap_uid_array_free (src);
                imap_uid_array_free (dest);
@@ -2667,6 +2666,8 @@ handle_copyuid (CamelImapResponse *response,
        g_warning ("Bad COPYUID response from server");
 }
 
+
+/* Call with lock held on destination folder cache */
 static void
 handle_copyuid_copy_user_tags (CamelImapResponse *response,
                                CamelFolder *source,
@@ -2708,12 +2709,7 @@ handle_copyuid_copy_user_tags (CamelImapResponse *response,
        dest = imap_uid_set_to_array (destination->summary, destset);
 
        if (src && dest && src->len == dest->len) {
-               /* We don't have to worry about deadlocking on the
-                * cache locks here, because we've got the store's
-                * command lock too, so no one else could be here.
-                */
                CAMEL_IMAP_FOLDER_REC_LOCK (source, cache_lock);
-               CAMEL_IMAP_FOLDER_REC_LOCK (destination, cache_lock);
                for (i = 0; i < src->len; i++) {
                        CamelMessageInfo *mi = camel_folder_get_message_info (source, src->pdata[i]);
 
@@ -2729,7 +2725,6 @@ handle_copyuid_copy_user_tags (CamelImapResponse *response,
                        }
                }
                CAMEL_IMAP_FOLDER_REC_UNLOCK (source, cache_lock);
-               CAMEL_IMAP_FOLDER_REC_UNLOCK (destination, cache_lock);
 
                imap_uid_array_free (src);
                imap_uid_array_free (dest);
@@ -2823,6 +2818,7 @@ do_copy (CamelFolder *source,
                        /* returns only 'A00012 OK UID XGWMOVE completed' '* 2 XGWMOVE' so nothing useful */
                        camel_imap_response_free (store, response);
                } else {
+                       CAMEL_IMAP_FOLDER_REC_LOCK (destination, cache_lock);
                        response = camel_imap_command (
                                store, source, cancellable, &local_error,
                                "UID COPY %s %F", uidset, full_name);
@@ -2833,6 +2829,7 @@ do_copy (CamelFolder *source,
                                        response, source, destination,
                                        cancellable);
                        camel_imap_response_free (store, response);
+                       CAMEL_IMAP_FOLDER_REC_UNLOCK (destination, cache_lock);
                }
 
                if (local_error == NULL && delete_originals && (mark_moved || !trash_path)) {