From: Timo Sirainen Date: Sat, 13 Dec 2008 08:36:13 +0000 (+0200) Subject: mbox locking fixes. X-Git-Tag: 1.2.alpha5~28 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=73247459cf41eb1e5ae5bc61354db46d3b05ee75;p=thirdparty%2Fdovecot%2Fcore.git 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 f83de81e11..6ff5811529 100644 --- 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 45f53b0881..f234fcb1d9 100644 --- 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 d77dfa9b2f..38abe8404b 100644 --- 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)