[evolution-data-server/gnome-3-28] Bug 767683 - Crash when updating Maildir 'changes' structure

[Milan Crha <mcrha src gnome org>]

commit f93fd6c6597f2169b7d777e4df7eb68e95137256
Author: Milan Crha <mcrha redhat com>
Date:   Mon Mar 12 13:51:59 2018 +0100

    Bug 767683 - Crash when updating Maildir 'changes' structure

 src/camel/providers/local/camel-local-folder.c   |   70 ++++++++++++++++++---
 src/camel/providers/local/camel-local-folder.h   |    5 ++
 src/camel/providers/local/camel-local-private.h  |    1 +
 src/camel/providers/local/camel-maildir-folder.c |   34 +++++------
 src/camel/providers/local/camel-mbox-folder.c    |   40 ++++++++-----
 src/camel/providers/local/camel-mh-folder.c      |   17 +++---
 6 files changed, 115 insertions(+), 52 deletions(-)
---
diff --git a/src/camel/providers/local/camel-local-folder.c b/src/camel/providers/local/camel-local-folder.c
index 6666b3d..e71b7e6 100644
--- a/src/camel/providers/local/camel-local-folder.c
+++ b/src/camel/providers/local/camel-local-folder.c
@@ -106,9 +106,11 @@ local_folder_dispose (GObject *object)
                /* Something can hold the reference to the folder longer than
                   the parent store is alive, thus count with it. */
                if (camel_folder_get_parent_store (folder)) {
+                       camel_local_folder_lock_changes (local_folder);
                        camel_local_summary_sync (
                                CAMEL_LOCAL_SUMMARY (camel_folder_get_folder_summary (folder)),
                                FALSE, local_folder->changes, NULL, NULL);
+                       camel_local_folder_unlock_changes (local_folder);
                }
        }
 
@@ -136,6 +138,7 @@ local_folder_finalize (GObject *object)
        camel_folder_change_info_free (local_folder->changes);
 
        g_mutex_clear (&local_folder->priv->search_lock);
+       g_rec_mutex_clear (&local_folder->priv->changes_lock);
 
        /* Chain up to parent's finalize() method. */
        G_OBJECT_CLASS (camel_local_folder_parent_class)->finalize (object);
@@ -409,15 +412,16 @@ local_folder_refresh_info_sync (CamelFolder *folder,
        need_summary_check = camel_local_store_get_need_summary_check (
                CAMEL_LOCAL_STORE (parent_store));
 
+       camel_local_folder_lock_changes (lf);
        if (need_summary_check &&
-           camel_local_summary_check ((CamelLocalSummary *) camel_folder_get_folder_summary (folder), 
lf->changes, cancellable, error) == -1)
+           camel_local_summary_check ((CamelLocalSummary *) camel_folder_get_folder_summary (folder), 
lf->changes, cancellable, error) == -1) {
+               camel_local_folder_unlock_changes (lf);
                return FALSE;
-
-       if (camel_folder_change_info_changed (lf->changes)) {
-               camel_folder_changed (folder, lf->changes);
-               camel_folder_change_info_clear (lf->changes);
        }
 
+       camel_local_folder_unlock_changes (lf);
+       camel_local_folder_claim_changes (lf);
+
        return TRUE;
 }
 
@@ -432,8 +436,12 @@ local_folder_synchronize_sync (CamelFolder *folder,
 
        d (printf ("local sync '%s' , expunge=%s\n", folder->full_name, expunge?"true":"false"));
 
-       if (camel_local_folder_lock (lf, CAMEL_LOCK_WRITE, error) == -1)
+       camel_local_folder_lock_changes (lf);
+
+       if (camel_local_folder_lock (lf, CAMEL_LOCK_WRITE, error) == -1) {
+               camel_local_folder_unlock_changes (lf);
                return FALSE;
+       }
 
        camel_object_state_write (CAMEL_OBJECT (lf));
 
@@ -443,10 +451,8 @@ local_folder_synchronize_sync (CamelFolder *folder,
                expunge, lf->changes, cancellable, error) == 0);
        camel_local_folder_unlock (lf);
 
-       if (camel_folder_change_info_changed (lf->changes)) {
-               camel_folder_changed (folder, lf->changes);
-               camel_folder_change_info_clear (lf->changes);
-       }
+       camel_local_folder_unlock_changes (lf);
+       camel_local_folder_claim_changes (lf);
 
        return success;
 }
@@ -515,6 +521,7 @@ camel_local_folder_init (CamelLocalFolder *local_folder)
 
        local_folder->priv = CAMEL_LOCAL_FOLDER_GET_PRIVATE (local_folder);
        g_mutex_init (&local_folder->priv->search_lock);
+       g_rec_mutex_init (&local_folder->priv->changes_lock);
 
        camel_folder_set_flags (folder, camel_folder_get_flags (folder) | 
CAMEL_FOLDER_HAS_SUMMARY_CAPABILITY);
 
@@ -607,6 +614,8 @@ camel_local_folder_construct (CamelLocalFolder *lf,
 #endif
        }
 #endif
+       camel_local_folder_lock_changes (lf);
+
        lf->changes = camel_folder_change_info_new ();
 
        /* TODO: Remove the following line, it is a temporary workaround to remove
@@ -645,12 +654,15 @@ camel_local_folder_construct (CamelLocalFolder *lf,
                    camel_local_summary_check ((CamelLocalSummary *) camel_folder_get_folder_summary 
(folder), lf->changes, cancellable, error) == 0) {
                        /* we sync here so that any hard work setting up the folder isn't lost */
                        if (camel_local_summary_sync ((CamelLocalSummary *) camel_folder_get_folder_summary 
(folder), FALSE, lf->changes, cancellable, error) == -1) {
+                               camel_local_folder_unlock_changes (lf);
                                g_object_unref (folder);
                                return NULL;
                        }
                }
        }
 
+       camel_local_folder_unlock_changes (lf);
+
        /* TODO: This probably shouldn't be here? */
        if ((flags & CAMEL_STORE_FOLDER_CREATE) != 0) {
                CamelFolderInfo *fi;
@@ -737,3 +749,41 @@ set_cannot_get_message_ex (GError **error,
                _("Cannot get message %s from folder %s\n%s"),
                msgID, folder_path, detailErr);
 }
+
+void
+camel_local_folder_lock_changes (CamelLocalFolder *lf)
+{
+       g_return_if_fail (CAMEL_IS_LOCAL_FOLDER (lf));
+
+       g_rec_mutex_lock (&lf->priv->changes_lock);
+}
+
+void
+camel_local_folder_unlock_changes (CamelLocalFolder *lf)
+{
+       g_return_if_fail (CAMEL_IS_LOCAL_FOLDER (lf));
+
+       g_rec_mutex_unlock (&lf->priv->changes_lock);
+}
+
+void
+camel_local_folder_claim_changes (CamelLocalFolder *lf)
+{
+       CamelFolderChangeInfo *changes = NULL;
+
+       g_return_if_fail (CAMEL_IS_LOCAL_FOLDER (lf));
+
+       camel_local_folder_lock_changes (lf);
+
+       if (lf->changes && camel_folder_change_info_changed (lf->changes)) {
+               changes = lf->changes;
+               lf->changes = camel_folder_change_info_new ();
+       }
+
+       camel_local_folder_unlock_changes (lf);
+
+       if (changes) {
+               camel_folder_changed (CAMEL_FOLDER (lf), changes);
+               camel_folder_change_info_free (changes);
+       }
+}
diff --git a/src/camel/providers/local/camel-local-folder.h b/src/camel/providers/local/camel-local-folder.h
index e09c410..cf90f52 100644
--- a/src/camel/providers/local/camel-local-folder.h
+++ b/src/camel/providers/local/camel-local-folder.h
@@ -120,6 +120,11 @@ void               set_cannot_get_message_ex       (GError **error,
                                                 const gchar *msgID,
                                                 const gchar *folder_path,
                                                 const gchar *detailErr);
+void           camel_local_folder_lock_changes (CamelLocalFolder *lf);
+void           camel_local_folder_unlock_changes
+                                               (CamelLocalFolder *lf);
+void           camel_local_folder_claim_changes
+                                               (CamelLocalFolder *lf);
 
 G_END_DECLS
 
diff --git a/src/camel/providers/local/camel-local-private.h b/src/camel/providers/local/camel-local-private.h
index 71eebc8..0405686 100644
--- a/src/camel/providers/local/camel-local-private.h
+++ b/src/camel/providers/local/camel-local-private.h
@@ -31,6 +31,7 @@ G_BEGIN_DECLS
 
 struct _CamelLocalFolderPrivate {
        GMutex search_lock;     /* for locking the search object */
+       GRecMutex changes_lock; /* for locking changes member */
 };
 
 #define CAMEL_LOCAL_FOLDER_LOCK(f, l) \
diff --git a/src/camel/providers/local/camel-maildir-folder.c 
b/src/camel/providers/local/camel-maildir-folder.c
index 494b6fb..9e6b259 100644
--- a/src/camel/providers/local/camel-maildir-folder.c
+++ b/src/camel/providers/local/camel-maildir-folder.c
@@ -169,14 +169,21 @@ maildir_folder_append_message_sync (CamelFolder *folder,
 
        d (printf ("Appending message\n"));
 
+       camel_local_folder_lock_changes (lf);
+
        /* If we can't lock, don't do anything */
-       if (!lf || camel_local_folder_lock (lf, CAMEL_LOCK_WRITE, error) == -1)
+       if (!lf || camel_local_folder_lock (lf, CAMEL_LOCK_WRITE, error) == -1) {
+               camel_local_folder_unlock_changes (lf);
                return FALSE;
+       }
 
        /* add it to the summary/assign the uid, etc */
        mi = camel_local_summary_add (
                CAMEL_LOCAL_SUMMARY (camel_folder_get_folder_summary (folder)),
                message, info, lf->changes, error);
+
+       camel_local_folder_unlock_changes (lf);
+
        if (mi == NULL)
                goto check_changed;
 
@@ -245,10 +252,7 @@ maildir_folder_append_message_sync (CamelFolder *folder,
  check_changed:
        camel_local_folder_unlock (lf);
 
-       if (camel_folder_change_info_changed (lf->changes)) {
-               camel_folder_changed (folder, lf->changes);
-               camel_folder_change_info_clear (lf->changes);
-       }
+       camel_local_folder_claim_changes (lf);
 
        g_clear_object (&mi);
 
@@ -301,10 +305,7 @@ maildir_folder_get_message_sync (CamelFolder *folder,
 
        camel_local_folder_unlock (lf);
 
-       if (lf && camel_folder_change_info_changed (lf->changes)) {
-               camel_folder_changed (folder, lf->changes);
-               camel_folder_change_info_clear (lf->changes);
-       }
+       camel_local_folder_claim_changes (lf);
 
        return message;
 }
@@ -381,12 +382,16 @@ maildir_folder_transfer_messages_to_sync (CamelFolder *source,
                                        camel_message_info_set_flags (info, CAMEL_MESSAGE_JUNK, 0);
                                camel_folder_summary_add (camel_folder_get_folder_summary (dest), clone, 
FALSE);
 
+                               camel_local_folder_lock_changes (df);
                                camel_folder_change_info_add_uid (df->changes, camel_message_info_get_uid 
(clone));
+                               camel_local_folder_unlock_changes (df);
 
                                camel_folder_set_message_flags (
                                        source, uid, CAMEL_MESSAGE_DELETED |
                                        CAMEL_MESSAGE_SEEN, ~0);
+                               camel_local_folder_lock_changes (lf);
                                camel_folder_change_info_remove_uid (lf->changes, camel_message_info_get_uid 
(info));
+                               camel_local_folder_unlock_changes (lf);
                                camel_folder_summary_remove (camel_folder_get_folder_summary (source), info);
                                g_clear_object (&clone);
                        }
@@ -397,15 +402,8 @@ maildir_folder_transfer_messages_to_sync (CamelFolder *source,
                        g_free (new_filename);
                }
 
-               if (lf && camel_folder_change_info_changed (lf->changes)) {
-                       camel_folder_changed (source, lf->changes);
-                       camel_folder_change_info_clear (lf->changes);
-               }
-
-               if (df && camel_folder_change_info_changed (df->changes)) {
-                       camel_folder_changed (dest, df->changes);
-                       camel_folder_change_info_clear (df->changes);
-               }
+               camel_local_folder_claim_changes (lf);
+               camel_local_folder_claim_changes (df);
 
                camel_folder_thaw (source);
                camel_folder_thaw (dest);
diff --git a/src/camel/providers/local/camel-mbox-folder.c b/src/camel/providers/local/camel-mbox-folder.c
index 094772c..3300f89 100644
--- a/src/camel/providers/local/camel-mbox-folder.c
+++ b/src/camel/providers/local/camel-mbox-folder.c
@@ -109,13 +109,18 @@ mbox_folder_get_filename (CamelFolder *folder,
 
        d (printf ("Getting message %s\n", uid));
 
+       camel_local_folder_lock_changes (lf);
+
        /* lock the folder first, burn if we can't, need write lock for summary check */
-       if (camel_local_folder_lock (lf, CAMEL_LOCK_WRITE, error) == -1)
+       if (camel_local_folder_lock (lf, CAMEL_LOCK_WRITE, error) == -1) {
+               camel_local_folder_unlock_changes (lf);
                return NULL;
+       }
 
        /* check for new messages always */
        if (camel_local_summary_check ((CamelLocalSummary *) camel_folder_get_folder_summary (folder), 
lf->changes, NULL, error) == -1) {
                camel_local_folder_unlock (lf);
+               camel_local_folder_unlock_changes (lf);
                return NULL;
        }
 
@@ -138,6 +143,7 @@ mbox_folder_get_filename (CamelFolder *folder,
 
        /* and unlock now we're finished with it */
        camel_local_folder_unlock (lf);
+       camel_local_folder_unlock_changes (lf);
 
        return filename;
 }
@@ -162,9 +168,14 @@ mbox_folder_append_message_sync (CamelFolder *folder,
 #if 0
        gchar *xev;
 #endif
+
+       camel_local_folder_lock_changes (lf);
+
        /* If we can't lock, dont do anything */
-       if (camel_local_folder_lock (lf, CAMEL_LOCK_WRITE, error) == -1)
+       if (camel_local_folder_lock (lf, CAMEL_LOCK_WRITE, error) == -1) {
+               camel_local_folder_unlock_changes (lf);
                return FALSE;
+       }
 
        d (printf ("Appending message\n"));
 
@@ -240,10 +251,8 @@ mbox_folder_append_message_sync (CamelFolder *folder,
        /* unlock as soon as we can */
        camel_local_folder_unlock (lf);
 
-       if (camel_folder_change_info_changed (lf->changes)) {
-               camel_folder_changed (folder, lf->changes);
-               camel_folder_change_info_clear (lf->changes);
-       }
+       camel_local_folder_unlock_changes (lf);
+       camel_local_folder_claim_changes (lf);
 
        if (appended_uid)
                *appended_uid = g_strdup(camel_message_info_get_uid(mi));
@@ -289,11 +298,9 @@ fail:
        /* make sure we unlock the folder - before we start triggering events into appland */
        camel_local_folder_unlock (lf);
 
+       camel_local_folder_unlock_changes (lf);
        /* cascade the changes through, anyway, if there are any outstanding */
-       if (camel_folder_change_info_changed (lf->changes)) {
-               camel_folder_changed (folder, lf->changes);
-               camel_folder_change_info_clear (lf->changes);
-       }
+       camel_local_folder_claim_changes (lf);
 
        g_clear_object (&mi);
 
@@ -316,13 +323,18 @@ mbox_folder_get_message_sync (CamelFolder *folder,
 
        d (printf ("Getting message %s\n", uid));
 
+       camel_local_folder_lock_changes (lf);
+
        /* lock the folder first, burn if we can't, need write lock for summary check */
-       if (camel_local_folder_lock (lf, CAMEL_LOCK_WRITE, error) == -1)
+       if (camel_local_folder_lock (lf, CAMEL_LOCK_WRITE, error) == -1) {
+               camel_local_folder_unlock_changes (lf);
                return NULL;
+       }
 
        /* check for new messages always */
        if (camel_local_summary_check ((CamelLocalSummary *) camel_folder_get_folder_summary (folder), 
lf->changes, cancellable, error) == -1) {
                camel_local_folder_unlock (lf);
+               camel_local_folder_unlock_changes (lf);
                return NULL;
        }
 
@@ -404,15 +416,13 @@ retry:
 fail:
        /* and unlock now we're finished with it */
        camel_local_folder_unlock (lf);
+       camel_local_folder_unlock_changes (lf);
 
        if (parser)
                g_object_unref (parser);
 
        /* use the opportunity to notify of changes (particularly if we had a rebuild) */
-       if (camel_folder_change_info_changed (lf->changes)) {
-               camel_folder_changed (folder, lf->changes);
-               camel_folder_change_info_clear (lf->changes);
-       }
+       camel_local_folder_claim_changes (lf);
 
        return message;
 }
diff --git a/src/camel/providers/local/camel-mh-folder.c b/src/camel/providers/local/camel-mh-folder.c
index f3cc140..aa21a1b 100644
--- a/src/camel/providers/local/camel-mh-folder.c
+++ b/src/camel/providers/local/camel-mh-folder.c
@@ -66,12 +66,17 @@ mh_folder_append_message_sync (CamelFolder *folder,
 
        d (printf ("Appending message\n"));
 
+       camel_local_folder_lock_changes (lf);
+
        /* If we can't lock, don't do anything */
-       if (!lf || camel_local_folder_lock (lf, CAMEL_LOCK_WRITE, error) == -1)
+       if (!lf || camel_local_folder_lock (lf, CAMEL_LOCK_WRITE, error) == -1) {
+               camel_local_folder_unlock_changes (lf);
                return FALSE;
+       }
 
        /* add it to the summary/assign the uid, etc */
        mi = camel_local_summary_add ((CamelLocalSummary *) camel_folder_get_folder_summary (folder), 
message, info, lf->changes, error);
+       camel_local_folder_unlock_changes (lf);
        if (mi == NULL)
                goto check_changed;
 
@@ -123,10 +128,7 @@ mh_folder_append_message_sync (CamelFolder *folder,
  check_changed:
        camel_local_folder_unlock (lf);
 
-       if (camel_folder_change_info_changed (lf->changes)) {
-               camel_folder_changed (folder, lf->changes);
-               camel_folder_change_info_clear (lf->changes);
-       }
+       camel_local_folder_claim_changes (lf);
 
        g_clear_object (&mi);
 
@@ -189,10 +191,7 @@ mh_folder_get_message_sync (CamelFolder *folder,
 
        camel_local_folder_unlock (lf);
 
-       if (camel_folder_change_info_changed (lf->changes)) {
-               camel_folder_changed (folder, lf->changes);
-               camel_folder_change_info_clear (lf->changes);
-       }
+       camel_local_folder_claim_changes (lf);
 
        return message;
 }