mbox locking fixes.

--HG--
branch : HEAD

diff --git a/src/lib-storage/index/mbox/mbox-mail.c b/src/lib-storage/index/mbox/mbox-mail.c
index f83de81e110a8427b7aa9fbdb65aa96be85b77ab..6ff5811529d0314ce8f934650a0210fd881fc415 100644 (file)
--- a/src/lib-storage/index/mbox/mbox-mail.c
+++ b/src/lib-storage/index/mbox/mbox-mail.c
@@ -25,7 +25,6 @@ static void mbox_prepare_resync(struct index_mail *mail)
                if (mbox->mbox_lock_id == t->mbox_lock_id)
                        t->mbox_lock_id = 0;
                (void)mbox_unlock(mbox, mbox->mbox_lock_id);
-               mbox->mbox_lock_id = 0;
                i_assert(mbox->mbox_lock_type == F_UNLCK);
        }
 }
@@ -49,10 +48,16 @@ static int mbox_mail_seek(struct index_mail *mail)
        }
 
        for (try = 0; try < 2; try++) {
+               if ((sync_flags & MBOX_SYNC_FORCE_SYNC) != 0) {
+                       /* dirty offsets are broken. make sure we can sync. */
+                       mbox_prepare_resync(mail);
+               }
                if (mbox->mbox_lock_type == F_UNLCK) {
                        sync_flags |= MBOX_SYNC_LOCK_READING;
                        if (mbox_sync(mbox, sync_flags) < 0)
                                return -1;
+                       t->mbox_lock_id = mbox->mbox_lock_id;
+                       i_assert(t->mbox_lock_id != 0);
 
                        /* refresh index file after mbox has been locked to
                           make sure we get only up-to-date mbox offsets. */
@@ -62,13 +67,13 @@ static int mbox_mail_seek(struct index_mail *mail)
                        }
 
                        i_assert(mbox->mbox_lock_type != F_UNLCK);
-                       t->mbox_lock_id = mbox->mbox_lock_id;
-               } else if ((sync_flags & MBOX_SYNC_FORCE_SYNC) != 0) {
-                       /* dirty offsets are broken and mbox is write-locked.
-                          sync it to update offsets. */
-                       mbox_prepare_resync(mail);
-                       if (mbox_sync(mbox, sync_flags) < 0)
-                               return -1;
+               } else if (t->mbox_lock_id == 0) {
+                       /* file is already locked by another transaction, but
+                          we must keep it locked for the entire transaction,
+                          so increase the lock counter. */
+                       if (mbox_lock(mbox, mbox->mbox_lock_type,
+                                     &t->mbox_lock_id) < 0)
+                               i_unreached();
                }
 
                if (mbox_file_open_stream(mbox) < 0)

diff --git a/src/lib-storage/index/mbox/mbox-sync.c b/src/lib-storage/index/mbox/mbox-sync.c
index 45f53b08812d20d27dab774c2e008c386c4778a4..f234fcb1d902264e476ba03770e3d09d4fe7c0bd 100644 (file)
--- a/src/lib-storage/index/mbox/mbox-sync.c
+++ b/src/lib-storage/index/mbox/mbox-sync.c
@@ -1661,14 +1661,14 @@ static void mbox_sync_context_free(struct mbox_sync_context *sync_ctx)
        array_free(&sync_ctx->mails);
 }
 
-static int mbox_sync_int(struct mbox_mailbox *mbox, enum mbox_sync_flags flags)
+static int mbox_sync_int(struct mbox_mailbox *mbox, enum mbox_sync_flags flags,
+                        unsigned int *lock_id)
 {
        struct mail_index_sync_ctx *index_sync_ctx;
        struct mail_index_view *sync_view;
        struct mail_index_transaction *trans;
        struct mbox_sync_context sync_ctx;
        enum mail_index_sync_flags sync_flags;
-       unsigned int lock_id = 0;
        int ret, changed;
        bool delay_writes;
 
@@ -1680,7 +1680,7 @@ static int mbox_sync_int(struct mbox_mailbox *mbox, enum mbox_sync_flags flags)
                flags |= MBOX_SYNC_UNDIRTY;
 
        if ((flags & MBOX_SYNC_LOCK_READING) != 0) {
-               if (mbox_lock(mbox, F_RDLCK, &lock_id) <= 0)
+               if (mbox_lock(mbox, F_RDLCK, lock_id) <= 0)
                        return -1;
        }
 
@@ -1691,11 +1691,8 @@ static int mbox_sync_int(struct mbox_mailbox *mbox, enum mbox_sync_flags flags)
                changed = 1;
        } else {
                bool leave_dirty = (flags & MBOX_SYNC_UNDIRTY) == 0;
-               if ((changed = mbox_sync_has_changed(mbox, leave_dirty)) < 0) {
-                       if ((flags & MBOX_SYNC_LOCK_READING) != 0)
-                               (void)mbox_unlock(mbox, lock_id);
+               if ((changed = mbox_sync_has_changed(mbox, leave_dirty)) < 0)
                        return -1;
-               }
        }
 
        if ((flags & MBOX_SYNC_LOCK_READING) != 0) {
@@ -1705,8 +1702,8 @@ static int mbox_sync_int(struct mbox_mailbox *mbox, enum mbox_sync_flags flags)
                        return 0;
 
                /* have to sync to make sure offsets have stayed the same */
-               (void)mbox_unlock(mbox, lock_id);
-               lock_id = 0;
+               (void)mbox_unlock(mbox, *lock_id);
+               *lock_id = 0;
        }
 
        /* reopen input stream to make sure it has nothing buffered */
@@ -1720,12 +1717,12 @@ again:
                   don't have much choice either (well, easy ones anyway). */
                int lock_type = mbox->mbox_readonly ? F_RDLCK : F_WRLCK;
 
-               if ((ret = mbox_lock(mbox, lock_type, &lock_id)) <= 0) {
+               if ((ret = mbox_lock(mbox, lock_type, lock_id)) <= 0) {
                        if (ret == 0 || lock_type == F_RDLCK)
                                return -1;
 
                        /* try as read-only */
-                       if (mbox_lock(mbox, F_RDLCK, &lock_id) <= 0)
+                       if (mbox_lock(mbox, F_RDLCK, lock_id) <= 0)
                                return -1;
                        mbox->mbox_readonly = TRUE;
                }
@@ -1750,8 +1747,6 @@ again:
        if (ret <= 0) {
                if (ret < 0)
                        mail_storage_set_index_error(&mbox->ibox);
-               if (lock_id != 0)
-                       (void)mbox_unlock(mbox, lock_id);
                return ret;
        }
 
@@ -1765,9 +1760,6 @@ again:
        if (!changed && !mail_index_sync_have_more(index_sync_ctx)) {
                /* nothing to do */
        nothing_to_do:
-               if (lock_id != 0)
-                       (void)mbox_unlock(mbox, lock_id);
-
                /* index may need to do internal syncing though, so commit
                   instead of rollbacking. */
                if (mail_index_sync_commit(&index_sync_ctx) < 0) {
@@ -1821,7 +1813,7 @@ again:
                }
        }
 
-       if (lock_id == 0) {
+       if (*lock_id == 0) {
                /* ok, we have something to do but no locks. we'll have to
                   restart syncing to avoid deadlocking. */
                mbox_sync_context_free(&sync_ctx);
@@ -1831,7 +1823,6 @@ again:
 
        if (mbox_file_open_stream(mbox) < 0) {
                mbox_sync_context_free(&sync_ctx);
-               (void)mbox_unlock(mbox, lock_id);
                return -1;
        }
 
@@ -1870,7 +1861,7 @@ again:
                }
        }
 
-       i_assert(lock_id != 0);
+       i_assert(*lock_id != 0);
 
        if ((mbox->storage->storage.flags &
             MAIL_STORAGE_FLAG_NFS_FLUSH_STORAGE) != 0 && mbox->mbox_fd != -1) {
@@ -1880,36 +1871,39 @@ again:
                }
        }
 
-       if (mbox->mbox_lock_type != F_RDLCK) {
-               /* drop to read lock */
-               unsigned int read_lock_id = 0;
-
-               if (mbox_lock(mbox, F_RDLCK, &read_lock_id) <= 0)
-                       ret = -1;
-               else {
-                       if (mbox_unlock(mbox, lock_id) < 0)
-                               ret = -1;
-                       lock_id = read_lock_id;
-               }
-       }
-
-       if ((flags & MBOX_SYNC_LOCK_READING) == 0) {
-               if (mbox_unlock(mbox, lock_id) < 0)
-                       ret = -1;
-       }
-
        mbox_sync_context_free(&sync_ctx);
        return ret;
 }
 
 int mbox_sync(struct mbox_mailbox *mbox, enum mbox_sync_flags flags)
 {
+       unsigned int lock_id = 0;
        int ret;
 
+       i_assert(mbox->mbox_lock_type != F_RDLCK);
+
        mbox->syncing = TRUE;
-       ret = mbox_sync_int(mbox, flags);
+       ret = mbox_sync_int(mbox, flags, &lock_id);
        mbox->syncing = FALSE;
 
+       if (lock_id != 0) {
+               if (ret < 0) {
+                       /* syncing failed, don't leave it locked */
+                       (void)mbox_unlock(mbox, lock_id);
+               } else if ((flags & MBOX_SYNC_LOCK_READING) == 0) {
+                       if (mbox_unlock(mbox, lock_id) < 0)
+                               ret = -1;
+               } else if (mbox->mbox_lock_type != F_RDLCK) {
+                       /* drop to read lock */
+                       unsigned int read_lock_id = 0;
+
+                       if (mbox_lock(mbox, F_RDLCK, &read_lock_id) <= 0)
+                               ret = -1;
+                       if (mbox_unlock(mbox, lock_id) < 0)
+                               ret = -1;
+               }
+       }
+
        if (mbox->ibox.box.v.sync_notify != NULL)
                mbox->ibox.box.v.sync_notify(&mbox->ibox.box, 0, 0);
        return ret;

diff --git a/src/lib-storage/index/mbox/mbox-transaction.c b/src/lib-storage/index/mbox/mbox-transaction.c
index d77dfa9b2f0c3b0f982b5e263b2b7f3d29f392d9..38abe8404b836a55264ae6474743991dc00336f7 100644 (file)
--- a/src/lib-storage/index/mbox/mbox-transaction.c
+++ b/src/lib-storage/index/mbox/mbox-transaction.c
@@ -50,6 +50,8 @@ static int mbox_transaction_commit(struct mail_index_transaction *t,
                if (mbox_unlock(mbox, lock_id) < 0)
                        ret = -1;
        }
+       i_assert(mbox->ibox.box.transaction_count > 0 ||
+                mbox->mbox_lock_type == F_UNLCK);
        return ret;
 }
 
@@ -64,6 +66,9 @@ static void mbox_transaction_rollback(struct mail_index_transaction *t)
        if (mt->mbox_lock_id != 0)
                (void)mbox_unlock(mbox, mt->mbox_lock_id);
        index_transaction_finish_rollback(&mt->ictx);
+
+       i_assert(mbox->ibox.box.transaction_count > 0 ||
+                mbox->mbox_lock_type == F_UNLCK);
 }
 
 static void mbox_transaction_created(struct mail_index_transaction *t)