[dovecot-cvs] dovecot/src/lib-index/mbox mbox-index.c,1.42,1.43 mbox-index.h,1.19,1.20 mbox-lock.c,1.10,1.11 mbox-rewrite.c,1.34,1.35 mbox-sync-full.c,1.2,1.3 mbox-sync.c,1.16,1.17

[cras at procontrol.fi]

Update of /home/cvs/dovecot/src/lib-index/mbox
In directory danu:/tmp/cvs-serv8907/lib-index/mbox

Modified Files:
	mbox-index.c mbox-index.h mbox-lock.c mbox-rewrite.c 
	mbox-sync-full.c mbox-sync.c 
Log Message:
Mostly mbox locking/syncing fixes. Still some problems though.



Index: mbox-index.c
===================================================================
RCS file: /home/cvs/dovecot/src/lib-index/mbox/mbox-index.c,v
retrieving revision 1.42
retrieving revision 1.43
diff -u -d -r1.42 -r1.43
--- mbox-index.c	27 Oct 2002 06:37:18 -0000	1.42
+++ mbox-index.c	2 Nov 2002 20:10:20 -0000	1.43
@@ -701,6 +701,20 @@
 	i_free(index);
 }
 
+static int mbox_index_set_lock(MailIndex *index, MailLockType lock_type)
+{
+	if (lock_type == MAIL_LOCK_UNLOCK)
+		(void)mbox_unlock(index);
+	return mail_index_set_lock(index, lock_type);
+}
+
+static int mbox_index_try_lock(MailIndex *index, MailLockType lock_type)
+{
+	if (lock_type == MAIL_LOCK_UNLOCK)
+		(void)mbox_unlock(index);
+	return mail_index_try_lock(index, lock_type);
+}
+
 static int mbox_index_update_flags(MailIndex *index, MailIndexRecord *rec,
 				   unsigned int seq, MailFlags flags,
 				   int external_change)
@@ -719,8 +733,8 @@
 	mail_index_open,
 	mail_index_open_or_create,
 	mbox_index_free,
-	mail_index_set_lock,
-	mail_index_try_lock,
+	mbox_index_set_lock,
+	mbox_index_try_lock,
 	mbox_index_rebuild,
 	mail_index_fsck,
 	mbox_index_sync,

Index: mbox-index.h
===================================================================
RCS file: /home/cvs/dovecot/src/lib-index/mbox/mbox-index.h,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -d -r1.19 -r1.20
--- mbox-index.h	27 Oct 2002 06:37:18 -0000	1.19
+++ mbox-index.h	2 Nov 2002 20:10:20 -0000	1.20
@@ -48,7 +48,7 @@
 
 MailIndex *mbox_index_alloc(const char *dir, const char *mbox_path);
 int mbox_index_rebuild(MailIndex *index);
-int mbox_index_sync(MailIndex *index);
+int mbox_index_sync(MailIndex *index, MailLockType lock_type, int *changes);
 int mbox_sync_full(MailIndex *index);
 IBuffer *mbox_open_mail(MailIndex *index, MailIndexRecord *rec,
 			time_t *internal_date, int *deleted);

Index: mbox-lock.c
===================================================================
RCS file: /home/cvs/dovecot/src/lib-index/mbox/mbox-lock.c,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -d -r1.10 -r1.11
--- mbox-lock.c	29 Oct 2002 06:46:47 -0000	1.10
+++ mbox-lock.c	2 Nov 2002 20:10:20 -0000	1.11
@@ -151,6 +151,10 @@
 {
 	struct stat st;
 
+	/* index must be locked before mbox file, to avoid deadlocks */
+	i_assert(index->lock_type != MAIL_LOCK_UNLOCK);
+
+	/* allow only unlock -> shared/exclusive or exclusive -> shared */
 	i_assert(lock_type == MAIL_LOCK_SHARED ||
 		 lock_type == MAIL_LOCK_EXCLUSIVE);
 	i_assert(lock_type != MAIL_LOCK_EXCLUSIVE ||
@@ -200,7 +204,6 @@
 	int failed;
 
 	index->mbox_lock_counter++;
-	index->mbox_lock_next_sync = MAIL_LOCK_UNLOCK;
 
 	if (index->mbox_lock_type == MAIL_LOCK_UNLOCK)
 		return TRUE;

Index: mbox-rewrite.c
===================================================================
RCS file: /home/cvs/dovecot/src/lib-index/mbox/mbox-rewrite.c,v
retrieving revision 1.34
retrieving revision 1.35
diff -u -d -r1.34 -r1.35
--- mbox-rewrite.c	29 Oct 2002 18:01:10 -0000	1.34
+++ mbox-rewrite.c	2 Nov 2002 20:10:20 -0000	1.35
@@ -406,9 +406,17 @@
 	unsigned int seq;
 	int tmp_fd, failed, dirty_found, rewrite;
 
-	i_assert(index->lock_type == MAIL_LOCK_EXCLUSIVE);
+	i_assert(index->lock_type == MAIL_LOCK_UNLOCK);
 
-	if ((index->header->flags & MAIL_INDEX_FLAG_DIRTY_MESSAGES) == 0) {
+	if (!index->set_lock(index, MAIL_LOCK_SHARED))
+		return FALSE;
+
+        rewrite = (index->header->flags & MAIL_INDEX_FLAG_DIRTY_MESSAGES);
+
+	if (!index->set_lock(index, MAIL_LOCK_UNLOCK))
+		return FALSE;
+
+	if (!rewrite) {
 		/* no need to rewrite */
 		return TRUE;
 	}
@@ -417,8 +425,7 @@
 	failed = TRUE; rewrite = FALSE;
 	do {
 		/* make sync() lock the file to prevent race conditions */
-                index->mbox_lock_next_sync = MAIL_LOCK_EXCLUSIVE;
-		if (!index->sync(index))
+		if (!index->sync_and_lock(index, MAIL_LOCK_EXCLUSIVE, NULL))
 			break;
 
 		inbuf = mbox_get_inbuf(index, 0, MAIL_LOCK_EXCLUSIVE);
@@ -442,6 +449,8 @@
 
 	if (!rewrite) {
 		(void)mbox_unlock(index);
+		if (!index->set_lock(index, MAIL_LOCK_EXCLUSIVE))
+			failed = TRUE;
 		if (inbuf != NULL)
 			i_buffer_unref(inbuf);
 		return !failed;
@@ -455,10 +464,9 @@
 	}
 	dirty_offset = 0;
 
-	//offset = hdr_size = body_size = 0; /* just to keep compiler happy */
-
-	t_push();
-	outbuf = o_buffer_create_file(tmp_fd, data_stack_pool, 8192, 0, FALSE);
+	/* note: we can't use data_stack_pool with outbuf because it's
+	   being written to inside t_push() .. t_pop() calls */
+	outbuf = o_buffer_create_file(tmp_fd, system_pool, 8192, 0, FALSE);
 
 	failed = FALSE; seq = 1;
 	rec = index->lookup(index, 1);
@@ -535,7 +543,6 @@
 
 	i_buffer_unref(inbuf);
 	o_buffer_unref(outbuf);
-	t_pop();
 
 	if (!failed) {
 		/* POSSIBLE DATA LOSS HERE. We're writing to the mbox file,
@@ -565,10 +572,13 @@
 		}
 	}
 
+	if (!index->set_lock(index, MAIL_LOCK_UNLOCK))
+		failed = TRUE;
+
 	(void)mbox_unlock(index);
 	(void)unlink(path);
 
 	if (close(tmp_fd) < 0)
 		index_file_set_syscall_error(index, path, "close()");
-	return failed;
+	return !failed;
 }

Index: mbox-sync-full.c
===================================================================
RCS file: /home/cvs/dovecot/src/lib-index/mbox/mbox-sync-full.c,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -d -r1.2 -r1.3
--- mbox-sync-full.c	27 Oct 2002 06:37:18 -0000	1.2
+++ mbox-sync-full.c	2 Nov 2002 20:10:21 -0000	1.3
@@ -47,6 +47,7 @@
 {
 	const void *part_data;
 	void *part_data_copy;
+	uoff_t virtual_size;
 	size_t size;
 
 	/* update FIELD_HDR_HEADER_SIZE */
@@ -54,10 +55,15 @@
 				&hdr_size->physical_size,
 				sizeof(hdr_size->physical_size));
 
+	/* reset FIELD_HDR_VIRTUAL_SIZE - we don't know it anymore */
+        virtual_size = (uoff_t)-1;
+	index->update_field_raw(update, DATA_HDR_VIRTUAL_SIZE,
+				&virtual_size, sizeof(virtual_size));
+
+	/* update DATA_FIELD_MESSAGEPART */
 	if ((rec->data_fields & DATA_FIELD_MESSAGEPART) == 0)
 		return TRUE;
 
-	/* update DATA_FIELD_MESSAGEPART */
 	part_data = index->lookup_field_raw(index, rec, DATA_FIELD_MESSAGEPART,
 					    &size);
 	if (part_data == NULL) {
@@ -77,10 +83,9 @@
 		return FALSE;
 	}
 
-	t_pop();
-
 	index->update_field_raw(update, DATA_FIELD_MESSAGEPART,
 				part_data_copy, size);
+	t_pop();
 	return TRUE;
 }
 
@@ -183,9 +188,6 @@
 	unsigned int seq;
 	int dirty;
 
-	if (!index->set_lock(index, MAIL_LOCK_EXCLUSIVE))
-		return FALSE;
-
 	mbox_skip_empty_lines(inbuf);
 
 	/* first make sure we start with a "From " line. If file is too
@@ -217,6 +219,10 @@
 			if (!mbox_skip_crlf(inbuf)) {
 				/* they just went and broke it, even while
 				   we had it locked. */
+				index_set_error(index,
+						"Error syncing mbox file %s: "
+						"LF not found where expected",
+						index->mbox_path);
 				return FALSE;
 			}
 		}
@@ -258,18 +264,20 @@
 int mbox_sync_full(MailIndex *index)
 {
 	IBuffer *inbuf;
-	int failed, unlock;
+	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;
 
-	unlock = index->mbox_lock_type == MAIL_LOCK_UNLOCK;
 	inbuf = mbox_get_inbuf(index, 0, MAIL_LOCK_SHARED);
 	if (inbuf == NULL)
 		return FALSE;
 
 	failed = !mbox_sync_buf(index, inbuf);
 	i_buffer_unref(inbuf);
-
-	if (unlock)
-		(void)mbox_unlock(index);
 
 	return !failed;
 }

Index: mbox-sync.c
===================================================================
RCS file: /home/cvs/dovecot/src/lib-index/mbox/mbox-sync.c,v
retrieving revision 1.16
retrieving revision 1.17
diff -u -d -r1.16 -r1.17
--- mbox-sync.c	27 Oct 2002 06:37:18 -0000	1.16
+++ mbox-sync.c	2 Nov 2002 20:10:21 -0000	1.17
@@ -33,9 +33,6 @@
 			offset += hdr_size + body_size;
 	}
 
-	if (index->lock_type == MAIL_LOCK_SHARED)
-		(void)mail_index_set_lock(index, MAIL_LOCK_UNLOCK);
-
 	if (offset > OFF_T_MAX) {
 		/* too large to fit in off_t */
 		return 0;
@@ -44,17 +41,16 @@
 	return offset;
 }
 
-int mbox_index_sync(MailIndex *index)
+int mbox_index_sync(MailIndex *index, MailLockType lock_type, int *changes)
 {
 	struct stat st;
-	MailLockType lock_type;
 	time_t index_mtime;
 	uoff_t filesize;
 
 	i_assert(index->lock_type != MAIL_LOCK_SHARED);
 
-	lock_type = index->mbox_lock_next_sync;
-	index->mbox_lock_next_sync = MAIL_LOCK_UNLOCK;
+	if (changes != NULL)
+		*changes = FALSE;
 	index->mbox_sync_counter = index->mbox_lock_counter;
 
 	if (index->fd == -1) {
@@ -79,34 +75,60 @@
                 mbox_file_close_fd(index);
 	}
 
-	if (lock_type != MAIL_LOCK_UNLOCK) {
-		if (!mbox_lock(index, lock_type))
+	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)
-		return TRUE;
+	if (index_mtime != st.st_mtime || index->mbox_size != filesize) {
+		mbox_file_close_inbuf(index);
 
-	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;
 
-	/* problem .. index->mbox_size points to data after the last message.
-	   that should be \n, \r\n, or end of file. modify filesize
-	   accordingly to allow any of the extra 0-2 bytes. 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 ||
-	    filesize == index->mbox_size+2)
-		index->mbox_size = filesize;
+		if (index->file_sync_stamp == 0 &&
+		    index->mbox_size == filesize) {
+			/* just opened the mailbox, and the file size is same
+			   as we expected. don't bother checking it any
+			   further. */
+		} 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);
+				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;
+			}
+		}
 
-	if (index->file_sync_stamp == 0 && index->mbox_size == filesize) {
-		/* just opened the mailbox, and the file size is same as
-		   we expected. don't bother checking it any further. */
 		index->file_sync_stamp = st.st_mtime;
-		return TRUE;
 	}
 
-	index->file_sync_stamp = st.st_mtime;
+	if (!index->set_lock(index, lock_type))
+		return FALSE;
 
-	/* file has changed, scan through the whole mbox */
-	return mbox_sync_full(index);
+	if (lock_type != MAIL_LOCK_UNLOCK) {
+		if (!mbox_lock(index, lock_type))
+			return FALSE;
+	} else {
+		if (!mbox_unlock(index))
+			return FALSE;
+	}
+
+	return TRUE;
 }