dovecot-2.2: lib-index: mail_cache_lock() partial rewrite.

dovecot at dovecot.org dovecot at dovecot.org
Tue Oct 7 15:47:52 UTC 2014


details:   http://hg.dovecot.org/dovecot-2.2/rev/8c84b69e7f69
changeset: 17902:8c84b69e7f69
user:      Timo Sirainen <tss at iki.fi>
date:      Tue Oct 07 18:47:09 2014 +0300
description:
lib-index: mail_cache_lock() partial rewrite.
require_same_reset_id is no longer needed, if it ever was. If we're locking
the cache file, we always want the latest one. The logic of locking in
general was somewhat confusing and it probably didn't always successfully
lock when it should have, because the reset_id happened to match an old
file.

diffstat:

 src/lib-index/mail-cache-fields.c      |    2 +-
 src/lib-index/mail-cache-private.h     |    2 +-
 src/lib-index/mail-cache-sync-update.c |    2 +-
 src/lib-index/mail-cache-transaction.c |    2 +-
 src/lib-index/mail-cache.c             |  127 +++++++++++++++-----------------
 5 files changed, 63 insertions(+), 72 deletions(-)

diffs (219 lines):

diff -r a10eea380073 -r 8c84b69e7f69 src/lib-index/mail-cache-fields.c
--- a/src/lib-index/mail-cache-fields.c	Tue Oct 07 02:54:35 2014 +0300
+++ b/src/lib-index/mail-cache-fields.c	Tue Oct 07 18:47:09 2014 +0300
@@ -531,7 +531,7 @@
 		return ret;
 	}
 
-	if (mail_cache_lock(cache, FALSE) <= 0)
+	if (mail_cache_lock(cache) <= 0)
 		return -1;
 
 	T_BEGIN {
diff -r a10eea380073 -r 8c84b69e7f69 src/lib-index/mail-cache-private.h
--- a/src/lib-index/mail-cache-private.h	Tue Oct 07 02:54:35 2014 +0300
+++ b/src/lib-index/mail-cache-private.h	Tue Oct 07 18:47:09 2014 +0300
@@ -212,7 +212,7 @@
 
 /* Explicitly lock the cache file. Returns -1 if error / timed out,
    1 if ok, 0 if cache is broken/doesn't exist */
-int mail_cache_lock(struct mail_cache *cache, bool require_same_reset_id);
+int mail_cache_lock(struct mail_cache *cache);
 int mail_cache_try_lock(struct mail_cache *cache);
 /* Returns -1 if cache is / just got corrupted, 0 if ok. */
 int mail_cache_unlock(struct mail_cache *cache);
diff -r a10eea380073 -r 8c84b69e7f69 src/lib-index/mail-cache-sync-update.c
--- a/src/lib-index/mail-cache-sync-update.c	Tue Oct 07 02:54:35 2014 +0300
+++ b/src/lib-index/mail-cache-sync-update.c	Tue Oct 07 18:47:09 2014 +0300
@@ -29,7 +29,7 @@
 	if (ctx == NULL)
 		return;
 
-	if (mail_cache_lock(cache, TRUE) > 0) {
+	if (mail_cache_lock(cache) > 0) {
 		/* update the record counts in the cache file's header. these
 		   are used to figure out when a cache file should be
 		   recreated and the old data dropped. */
diff -r a10eea380073 -r 8c84b69e7f69 src/lib-index/mail-cache-transaction.c
--- a/src/lib-index/mail-cache-transaction.c	Tue Oct 07 02:54:35 2014 +0300
+++ b/src/lib-index/mail-cache-transaction.c	Tue Oct 07 18:47:09 2014 +0300
@@ -251,7 +251,7 @@
 
 	mail_cache_transaction_open_if_needed(ctx);
 
-	if ((ret = mail_cache_lock(cache, FALSE)) <= 0) {
+	if ((ret = mail_cache_lock(cache)) <= 0) {
 		if (ret < 0)
 			return -1;
 
diff -r a10eea380073 -r 8c84b69e7f69 src/lib-index/mail-cache.c
--- a/src/lib-index/mail-cache.c	Tue Oct 07 02:54:35 2014 +0300
+++ b/src/lib-index/mail-cache.c	Tue Oct 07 18:47:09 2014 +0300
@@ -81,15 +81,15 @@
 {
 	struct stat st;
 
-	if (cache->file_cache == NULL)
-		return;
+	if (cache->file_cache != NULL)
+		file_cache_set_fd(cache->file_cache, cache->fd);
 
-	file_cache_set_fd(cache->file_cache, cache->fd);
-
-	if (fstat(cache->fd, &st) == 0)
-		(void)file_cache_set_size(cache->file_cache, st.st_size);
-	else if (!ESTALE_FSTAT(errno))
+	if (fstat(cache->fd, &st) == 0) {
+		if (cache->file_cache != NULL)
+			(void)file_cache_set_size(cache->file_cache, st.st_size);
+	} else if (!ESTALE_FSTAT(errno)) {
 		mail_cache_set_syscall_error(cache, "fstat()");
+	}
 
 	cache->st_ino = st.st_ino;
 	cache->st_dev = st.st_dev;
@@ -614,14 +614,13 @@
 }
 
 static int
-mail_cache_lock_full(struct mail_cache *cache, bool require_same_reset_id,
-		     bool nonblock)
+mail_cache_lock_full(struct mail_cache *cache, bool nonblock)
 {
 	const struct mail_index_ext *ext;
 	const void *data;
 	struct mail_index_view *iview;
 	uint32_t reset_id;
-	int i, ret;
+	int i;
 
 	i_assert(!cache->locked);
 
@@ -633,77 +632,69 @@
 	    cache->index->readonly)
 		return 0;
 
-	iview = mail_index_view_open(cache->index);
-	ext = mail_index_view_get_ext(iview, cache->ext_id);
-	reset_id = ext == NULL ? 0 : ext->reset_id;
-	mail_index_view_close(&iview);
-
-	if (ext == NULL && require_same_reset_id) {
-		/* cache not used */
-		return 0;
+	for (;;) {
+		if (mail_cache_lock_file(cache, nonblock) <= 0)
+			return -1;
+		i_assert(!MAIL_CACHE_IS_UNUSABLE(cache));
+		if (!mail_cache_need_reopen(cache)) {
+			/* locked the latest file */
+			break;
+		}
+		if (mail_cache_reopen_now(cache) <= 0) {
+			i_assert(cache->file_lock == NULL);
+			return -1;
+		}
+		i_assert(cache->file_lock == NULL);
+		/* okay, so it was just compressed. try again. */
 	}
 
-	for (i = 0; i < 3; i++) {
-		if (cache->hdr->file_seq != reset_id &&
-		    (require_same_reset_id || i == 0)) {
-			/* we want the latest cache file */
-			if (reset_id < cache->hdr->file_seq) {
-				/* either we're still waiting for index to
-				   catch up with a cache compression, or
-				   that catching up is never going to happen */
-				ret = 0;
-				break;
-			}
-			ret = mail_cache_reopen(cache);
-			if (ret < 0 || (ret == 0 && require_same_reset_id))
-				break;
+	/* now verify that the index reset_id matches the cache's file_seq */
+	for (i = 0; ; i++) {
+		iview = mail_index_view_open(cache->index);
+		ext = mail_index_view_get_ext(iview, cache->ext_id);
+		reset_id = ext == NULL ? 0 : ext->reset_id;
+		mail_index_view_close(&iview);
+
+		if (cache->hdr->file_seq == reset_id)
+			break;
+		/* mismatch. try refreshing index once. if that doesn't help,
+		   we can't use the cache. */
+		if (i > 0) {
+			mail_cache_unlock_file(cache);
+			return 0;
 		}
-
-		if ((ret = mail_cache_lock_file(cache, nonblock)) <= 0) {
-			ret = -1;
-			break;
-		}
-		cache->locked = TRUE;
-
-		if (cache->hdr->file_seq == reset_id ||
-		    !require_same_reset_id) {
-			/* got it */
-			break;
-		}
-
-		/* okay, so it was just compressed. try again. */
-		(void)mail_cache_unlock(cache);
-		ret = 0;
-	}
-
-	if (ret > 0) {
-		/* make sure our header is up to date */
-		if (cache->file_cache != NULL) {
-			file_cache_invalidate(cache->file_cache, 0,
-					      sizeof(struct mail_cache_header));
-		}
-		if (cache->read_buf != NULL)
-			buffer_set_used_size(cache->read_buf, 0);
-		if (mail_cache_map(cache, 0, 0, &data) > 0)
-			cache->hdr_copy = *cache->hdr;
-		else {
-			(void)mail_cache_unlock(cache);
-			ret = -1;
+		if (mail_index_refresh(cache->index) < 0) {
+			mail_cache_unlock_file(cache);
+			return -1;
 		}
 	}
 
-	i_assert((ret <= 0 && !cache->locked) || (ret > 0 && cache->locked));
-	return ret;
+	/* successfully locked - make sure our header is up to date */
+	cache->locked = TRUE;
+	cache->hdr_modified = FALSE;
+
+	if (cache->file_cache != NULL) {
+		file_cache_invalidate(cache->file_cache, 0,
+				      sizeof(struct mail_cache_header));
+	}
+	if (cache->read_buf != NULL)
+		buffer_set_used_size(cache->read_buf, 0);
+	if (mail_cache_map(cache, 0, 0, &data) <= 0) {
+		(void)mail_cache_unlock(cache);
+		return -1;
+	}
+	cache->hdr_copy = *cache->hdr;
+	return 1;
 }
 
-int mail_cache_lock(struct mail_cache *cache, bool require_same_reset_id)
+int mail_cache_lock(struct mail_cache *cache)
 {
-	return mail_cache_lock_full(cache, require_same_reset_id, FALSE);
+	return mail_cache_lock_full(cache, FALSE);
 }
 
 int mail_cache_try_lock(struct mail_cache *cache)
 {
-	return mail_cache_lock_full(cache, FALSE, TRUE);
+	return mail_cache_lock_full(cache, TRUE);
 }
 
 int mail_cache_unlock(struct mail_cache *cache)


More information about the dovecot-cvs mailing list