Mercurial > dovecot > original-hg > dovecot-1.2

changeset 570:2626acd3c6f4 HEAD

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
And more locking/syncing fixes. Now it's finally beginning to look sane again.
author Timo Sirainen <tss@iki.fi>
date Mon, 04 Nov 2002 06:47:40 +0200
parents cafe57140f5c
children 944dbdc61f3d
files src/lib-index/mail-index-open.c src/lib-index/mail-index.c src/lib-index/mail-index.h src/lib-index/mbox/mbox-rewrite.c src/lib-index/mbox/mbox-sync-full.c src/lib-index/mbox/mbox-sync.c src/lib-storage/index/index-copy.c src/lib-storage/index/index-expunge.c src/lib-storage/index/index-fetch.c src/lib-storage/index/index-status.c src/lib-storage/index/index-storage.h src/lib-storage/index/index-sync.c src/lib-storage/index/index-update-flags.c src/lib-storage/index/maildir/maildir-copy.c
diffstat 14 files changed, 112 insertions(+), 89 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-index/mail-index-open.c	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-index/mail-index-open.c	Mon Nov 04 06:47:40 2002 +0200
@@ -214,9 +214,17 @@
 
 	/* sync ourself - before updating cache and compression which
 	   may happen because of this. */
-	if (!index->sync_and_lock(index, MAIL_LOCK_EXCLUSIVE, NULL))
+	if (!index->sync_and_lock(index, MAIL_LOCK_SHARED, NULL))
 		return FALSE;
 
+	/* we never want to keep shared lock if syncing happens to set it.
+	   either exclusive or nothing (NOTE: drop it directly, not through
+	   index->set_lock() so mbox lock won't be affected). */
+	if (index->lock_type == MAIL_LOCK_SHARED) {
+		if (!mail_index_set_lock(index, MAIL_LOCK_UNLOCK))
+			return FALSE;
+	}
+
 	if (!fast && (index->header->flags & MAIL_INDEX_FLAG_CACHE_FIELDS)) {
 		/* need to update cached fields */
 		if (!mail_index_update_cache(index))
--- a/src/lib-index/mail-index.c	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-index/mail-index.c	Mon Nov 04 06:47:40 2002 +0200
@@ -339,7 +339,7 @@
 	debug_mprotect(index->mmap_base, index->mmap_full_length, index);
 
 	if (!mail_index_mmap_update(index)) {
-		(void)mail_index_set_lock(index, MAIL_LOCK_UNLOCK);
+		(void)index->set_lock(index, MAIL_LOCK_UNLOCK);
 		return FALSE;
 	}
 
@@ -377,7 +377,7 @@
 
 		if (lock_type == MAIL_LOCK_SHARED) {
 			/* drop exclusive lock */
-			return mail_index_set_lock(index, lock_type);
+			return index->set_lock(index, lock_type);
 		}
 	}
 
@@ -387,7 +387,7 @@
 		   removed. */
 		index->header->flags |= MAIL_INDEX_FLAG_FSCK;
 		if (!mail_index_fmdatasync(index, sizeof(MailIndexHeader))) {
-			(void)mail_index_set_lock(index, MAIL_LOCK_UNLOCK);
+			(void)index->set_lock(index, MAIL_LOCK_UNLOCK);
 			return FALSE;
 		}
 	}
--- a/src/lib-index/mail-index.h	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-index/mail-index.h	Mon Nov 04 06:47:40 2002 +0200
@@ -200,11 +200,11 @@
 	int (*fsck)(MailIndex *index);
 
 	/* Synchronize the index with the mailbox. Index must not have shared
-	   lock when calling this function. The lock_type specifies what
-	   locking state the index will be left, also locking mailbox file
-	   if needed. If changes is non-NULL, it's set to TRUE if any changes
-	   were noticed. */
-	int (*sync_and_lock)(MailIndex *index, MailLockType lock_type,
+	   lock when calling this function. The data_lock_type specifies what
+	   lock should be set to data file (mbox file). This function may
+	   leave the index in ANY locking state. If changes is non-NULL, it's
+	   set to TRUE if any changes were noticed. */
+	int (*sync_and_lock)(MailIndex *index, MailLockType data_lock_type,
 			     int *changes);
 
 	/* Returns the index header (never fails). The index needs to be
--- a/src/lib-index/mbox/mbox-rewrite.c	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-index/mbox/mbox-rewrite.c	Mon Nov 04 06:47:40 2002 +0200
@@ -424,7 +424,9 @@
 	tmp_fd = -1; inbuf = NULL;
 	failed = TRUE; rewrite = FALSE;
 	do {
-		/* make sync() lock the file to prevent race conditions */
+		if (!index->set_lock(index, MAIL_LOCK_EXCLUSIVE))
+			break;
+
 		if (!index->sync_and_lock(index, MAIL_LOCK_EXCLUSIVE, NULL))
 			break;
 
@@ -479,6 +481,13 @@
 				break;
 			}
 
+			if (offset < inbuf->v_offset) {
+				index_set_corrupted(index,
+						    "Invalid message offset");
+				failed = TRUE;
+				break;
+			}
+
 			if (offset + hdr_size + body_size > inbuf->v_size) {
 				index_set_corrupted(index,
 						    "Invalid message size");
--- a/src/lib-index/mbox/mbox-sync-full.c	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-index/mbox/mbox-sync-full.c	Mon Nov 04 06:47:40 2002 +0200
@@ -266,11 +266,7 @@
 	IBuffer *inbuf;
 	int failed;
 
-	if (index->lock_type == MAIL_LOCK_SHARED)
-		(void)mail_index_set_lock(index, MAIL_LOCK_UNLOCK);
-
-	if (!index->set_lock(index, MAIL_LOCK_EXCLUSIVE))
-		return FALSE;
+	i_assert(index->lock_type == MAIL_LOCK_EXCLUSIVE);
 
 	inbuf = mbox_get_inbuf(index, 0, MAIL_LOCK_SHARED);
 	if (inbuf == NULL)
--- a/src/lib-index/mbox/mbox-sync.c	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-index/mbox/mbox-sync.c	Mon Nov 04 06:47:40 2002 +0200
@@ -41,7 +41,32 @@
 	return offset;
 }
 
-int mbox_index_sync(MailIndex *index, MailLockType lock_type, int *changes)
+static int mbox_lock_and_sync_full(MailIndex *index,
+				   MailLockType data_lock_type)
+{
+        MailLockType lock_type;
+
+	/* syncing needs exclusive index lock and shared
+	   mbox lock, but if we'd want exclusive mbox lock
+	   we need to set it here already */
+	if (index->lock_type == MAIL_LOCK_SHARED)
+		(void)mail_index_set_lock(index, MAIL_LOCK_UNLOCK);
+
+	if (!index->set_lock(index, MAIL_LOCK_EXCLUSIVE))
+		return FALSE;
+
+	if (index->mbox_lock_type == MAIL_LOCK_UNLOCK) {
+		lock_type = data_lock_type == MAIL_LOCK_EXCLUSIVE ?
+			MAIL_LOCK_EXCLUSIVE : MAIL_LOCK_SHARED;
+		if (!mbox_lock(index, lock_type))
+			return FALSE;
+	}
+
+	return mbox_sync_full(index);
+}
+
+int mbox_index_sync(MailIndex *index, MailLockType data_lock_type,
+		    int *changes)
 {
 	struct stat st;
 	time_t index_mtime;
@@ -74,55 +99,37 @@
                 mbox_file_close_fd(index);
 	}
 
-	if (lock_type == MAIL_LOCK_EXCLUSIVE) {
-		/* if we know that we want exclusive lock, we might get
-		   it immediately to save extra lock changes */
-		if (!index->set_lock(index, MAIL_LOCK_EXCLUSIVE))
-			return FALSE;
-	}
-
 	if (index_mtime != st.st_mtime || index->mbox_size != filesize) {
 		mbox_file_close_inbuf(index);
 
-		/* problem .. index->mbox_size points to data after the last
-		   message. that should be \n or end of file. modify filesize
-		   accordingly to allow the extra byte. Don't actually bother
-		   to open the file and verify it, it'd just slow things.. */
 		index->mbox_size = get_indexed_mbox_size(index);
-		if (filesize == index->mbox_size+1)
-			index->mbox_size = filesize;
-
 		if (index->file_sync_stamp == 0 &&
-		    index->mbox_size == filesize) {
+		    index->mbox_size == filesize+1) {
 			/* just opened the mailbox, and the file size is same
 			   as we expected. don't bother checking it any
-			   further. */
+			   further. the +1 comes from the extra \n at end. */
 		} else {
 			if (changes != NULL)
 				*changes = TRUE;
 
-			/* file has changed, scan through the whole mbox */
-			if (!mbox_sync_full(index)) {
-				(void)index->set_lock(index, MAIL_LOCK_UNLOCK);
+			if (!mbox_lock_and_sync_full(index, data_lock_type))
 				return FALSE;
-			}
-
-			if (lock_type == MAIL_LOCK_EXCLUSIVE &&
-			    index->mbox_lock_type == MAIL_LOCK_SHARED) {
-				/* mbox_sync_full() left it */
-				if (!mbox_unlock(index))
-					return FALSE;
-			}
 		}
 
 		index->file_sync_stamp = st.st_mtime;
 	}
 
-	if (!index->set_lock(index, lock_type))
-		return FALSE;
+	/* we need some index lock to be able to lock mbox */
+	if (index->lock_type == MAIL_LOCK_UNLOCK) {
+		if (!index->set_lock(index, MAIL_LOCK_SHARED))
+			return FALSE;
+	}
 
-	if (lock_type != MAIL_LOCK_UNLOCK) {
-		if (!mbox_lock(index, lock_type))
+	if (data_lock_type == MAIL_LOCK_UNLOCK) {
+		if (!mbox_unlock(index))
+			return FALSE;
+	} else {
+		if (!mbox_lock(index, data_lock_type))
 			return FALSE;
 	}
 
--- a/src/lib-storage/index/index-copy.c	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-storage/index/index-copy.c	Mon Nov 04 06:47:40 2002 +0200
@@ -53,8 +53,8 @@
 		strcmp(destbox->name, box->name) == 0 ?
 		MAIL_LOCK_EXCLUSIVE : MAIL_LOCK_SHARED;
 
-	if (!ibox->index->sync_and_lock(ibox->index, lock_type, NULL))
-		return mail_storage_set_index_error(ibox);
+	if (!index_storage_sync_and_lock(ibox, TRUE, lock_type))
+		return FALSE;
 
 	ctx.custom_flags =
 		mail_custom_flags_list_get(ibox->index->custom_flags);
--- a/src/lib-storage/index/index-expunge.c	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-storage/index/index-expunge.c	Mon Nov 04 06:47:40 2002 +0200
@@ -70,13 +70,17 @@
 		return FALSE;
 	}
 
+	if (!ibox->index->set_lock(ibox->index, MAIL_LOCK_EXCLUSIVE))
+		return mail_storage_set_index_error(ibox);
+
 	if (!index_storage_sync_and_lock(ibox, FALSE, MAIL_LOCK_EXCLUSIVE))
 		return FALSE;
 
-	/* modifylog must be marked synced before expunging anything new */
-	failed = !index_storage_sync_modifylog(ibox, TRUE);
-
-	if (!failed)
+	/* modifylog must be marked synced before expunging
+	   anything new */
+	if (!index_storage_sync_modifylog(ibox, TRUE))
+		failed = TRUE;
+	else
 		failed = !ibox->expunge_locked(ibox, notify);
 
 	if (!ibox->index->set_lock(ibox->index, MAIL_LOCK_UNLOCK))
--- a/src/lib-storage/index/index-fetch.c	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-storage/index/index-fetch.c	Mon Nov 04 06:47:40 2002 +0200
@@ -362,21 +362,21 @@
 	}
 
 	/* need exclusive lock to update the \Seen flags */
-	if (!index_storage_sync_and_lock(ibox, TRUE, ctx.update_seen ?
-					 MAIL_LOCK_EXCLUSIVE :
-					 MAIL_LOCK_SHARED))
+	if (ctx.update_seen) {
+		if (!ibox->index->set_lock(ibox->index, MAIL_LOCK_EXCLUSIVE))
+			return mail_storage_set_index_error(ibox);
+	}
+
+	if (!index_storage_sync_and_lock(ibox, TRUE, MAIL_LOCK_SHARED))
 		return FALSE;
 
-	if (ctx.update_seen) {
+	if (ctx.update_seen &&
+	    ibox->index->header->messages_count ==
+	    ibox->index->header->seen_messages_count) {
 		/* if all messages are already seen, there's no point in
 		   keeping exclusive lock */
-		if (ibox->index->header->messages_count ==
-		    ibox->index->header->seen_messages_count) {
-			ctx.update_seen = FALSE;
-
-			(void)ibox->index->set_lock(ibox->index,
-						    MAIL_LOCK_SHARED);
-		}
+		ctx.update_seen = FALSE;
+		(void)ibox->index->set_lock(ibox->index, MAIL_LOCK_SHARED);
 	}
 
 	ctx.box = box;
--- a/src/lib-storage/index/index-status.c	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-storage/index/index-status.c	Mon Nov 04 06:47:40 2002 +0200
@@ -75,7 +75,7 @@
 
 	/* if we're doing STATUS for selected mailbox, we have to sync it
 	   first or STATUS reply may give different data */
-	if (!index_storage_sync_and_lock(ibox, TRUE, MAIL_LOCK_SHARED))
+	if (!index_storage_sync_and_lock(ibox, TRUE, MAIL_LOCK_UNLOCK))
 		return FALSE;
 
 	if (!index_storage_sync_modifylog(ibox, FALSE)) {
--- a/src/lib-storage/index/index-storage.h	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-storage/index/index-storage.h	Mon Nov 04 06:47:40 2002 +0200
@@ -38,7 +38,7 @@
 int index_storage_close(Mailbox *box);
 
 int index_storage_sync_and_lock(IndexMailbox *ibox, int sync_size,
-				MailLockType lock_type);
+				MailLockType data_lock_type);
 int index_storage_sync_modifylog(IndexMailbox *ibox, int hide_deleted);
 
 int index_mailbox_fix_custom_flags(IndexMailbox *ibox, MailFlags *flags,
--- a/src/lib-storage/index/index-sync.c	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-storage/index/index-sync.c	Mon Nov 04 06:47:40 2002 +0200
@@ -28,19 +28,14 @@
 }
 
 int index_storage_sync_and_lock(IndexMailbox *ibox, int sync_size,
-				MailLockType lock_type)
+				MailLockType data_lock_type)
 {
 	MailIndex *index = ibox->index;
-	int unlock, changes;
+	int changes, set_shared_lock;
 
-	if (lock_type != MAIL_LOCK_UNLOCK)
-		unlock = FALSE;
-	else {
-		unlock = TRUE;
-		lock_type = MAIL_LOCK_SHARED;
-	}
+        set_shared_lock = ibox->index->lock_type != MAIL_LOCK_EXCLUSIVE;
 
-	if (index->sync_and_lock(index, lock_type, &changes)) {
+	if (index->sync_and_lock(index, data_lock_type, &changes)) {
 		/* reset every time it has worked */
 		ibox->sent_diskspace_warning = FALSE;
 	} else {
@@ -60,12 +55,18 @@
 		index_reset_error(index);
 	}
 
+	if (set_shared_lock) {
+		/* just make sure we are locked, and that we drop our
+		   exclusive lock if it wasn't wanted originally */
+		if (!ibox->index->set_lock(ibox->index, MAIL_LOCK_SHARED)) {
+			(void)index->set_lock(index, MAIL_LOCK_UNLOCK);
+			return FALSE;
+		}
+	}
+
 	/* notify about changes in mailbox size. */
-	if (!changes) {
-		if (unlock)
-			(void)index->set_lock(index, MAIL_LOCK_UNLOCK);
+	if (!changes)
 		return TRUE; /* no changes - must be no new mail either */
-	}
 
 	if (sync_size)
 		index_storage_sync_size(ibox);
@@ -78,9 +79,6 @@
 			MAIL_CUSTOM_FLAGS_COUNT, ibox->sync_context);
 	}
 
-	if (unlock)
-		(void)index->set_lock(index, MAIL_LOCK_UNLOCK);
-
 	return TRUE;
 }
 
@@ -208,7 +206,7 @@
 	IndexMailbox *ibox = (IndexMailbox *) box;
 	int failed;
 
-	if (!index_storage_sync_and_lock(ibox, FALSE, MAIL_LOCK_SHARED))
+	if (!index_storage_sync_and_lock(ibox, FALSE, MAIL_LOCK_UNLOCK))
 		return FALSE;
 
 	if (!sync_expunges) {
@@ -218,9 +216,7 @@
 		failed = !index_storage_sync_modifylog(ibox, FALSE);
 	}
 
-	/* check size only if we're locked (== at least something changed) */
-	if (ibox->index->lock_type != MAIL_LOCK_UNLOCK)
-		index_storage_sync_size(ibox);
+	index_storage_sync_size(ibox);
 
 	if (!ibox->index->set_lock(ibox->index, MAIL_LOCK_UNLOCK))
 		return mail_storage_set_index_error(ibox);
--- a/src/lib-storage/index/index-update-flags.c	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-storage/index/index-update-flags.c	Mon Nov 04 06:47:40 2002 +0200
@@ -75,7 +75,10 @@
 	if (!index_mailbox_fix_custom_flags(ibox, &flags, custom_flags))
 		return FALSE;
 
-	if (!index_storage_sync_and_lock(ibox, TRUE, MAIL_LOCK_EXCLUSIVE))
+	if (!ibox->index->set_lock(ibox->index, MAIL_LOCK_EXCLUSIVE))
+		return mail_storage_set_index_error(ibox);
+
+	if (!index_storage_sync_and_lock(ibox, TRUE, MAIL_LOCK_UNLOCK))
 		return FALSE;
 
 	ctx.ibox = ibox;
--- a/src/lib-storage/index/maildir/maildir-copy.c	Sun Nov 03 12:11:47 2002 +0200
+++ b/src/lib-storage/index/maildir/maildir-copy.c	Mon Nov 04 06:47:40 2002 +0200
@@ -58,8 +58,8 @@
 	CopyHardContext ctx;
 	int ret;
 
-	if (!src->index->sync_and_lock(src->index, MAIL_LOCK_SHARED, NULL))
-		return mail_storage_set_index_error(src);
+	if (!index_storage_sync_and_lock(src, TRUE, MAIL_LOCK_SHARED))
+		return -1;
 
 	ctx.storage = src->box.storage;
 	ctx.dest = dest;