dovecot-1.1: Maildir saving: Fixed race condition bugs in uidlis...

[dovecot at dovecot.org]

details:   http://hg.dovecot.org/dovecot-1.1/rev/6bbe27c32cb8
changeset: 8261:6bbe27c32cb8
user:      Timo Sirainen <tss at iki.fi>
date:      Mon May 04 14:28:31 2009 -0400
description:
Maildir saving: Fixed race condition bugs in uidlist handling, causing files to be given new UIDs sometimes.

diffstat:

1 file changed, 18 insertions(+), 10 deletions(-)
src/lib-storage/index/maildir/maildir-uidlist.c |   28 ++++++++++++++---------

diffs (103 lines):

diff -r 6f8c536d76d0 -r 6bbe27c32cb8 src/lib-storage/index/maildir/maildir-uidlist.c
--- a/src/lib-storage/index/maildir/maildir-uidlist.c	Mon May 04 12:33:27 2009 -0400
+++ b/src/lib-storage/index/maildir/maildir-uidlist.c	Mon May 04 14:28:31 2009 -0400
@@ -91,6 +91,7 @@ struct maildir_uidlist {
 	unsigned int initial_hdr_read:1;
 	unsigned int initial_sync:1;
 	unsigned int retry_rewind:1;
+	unsigned int locked_refresh:1;
 };
 
 struct maildir_uidlist_sync_ctx {
@@ -171,6 +172,7 @@ static int maildir_uidlist_lock_timeout(
 	}
 
 	uidlist->lock_count++;
+	uidlist->locked_refresh = FALSE;
 
 	if (refresh) {
 		/* make sure we have the latest changes before
@@ -212,6 +214,7 @@ void maildir_uidlist_unlock(struct maild
 	if (--uidlist->lock_count > 0)
 		return;
 
+	uidlist->locked_refresh = FALSE;
 	(void)file_dotlock_delete(&uidlist->dotlock);
 }
 
@@ -808,6 +811,8 @@ int maildir_uidlist_refresh(struct maild
 	if (ret >= 0) {
 		uidlist->initial_read = TRUE;
 		uidlist->initial_hdr_read = TRUE;
+		if (UIDLIST_IS_LOCKED(uidlist))
+			uidlist->locked_refresh = TRUE;
 	}
         return ret;
 }
@@ -1015,7 +1020,10 @@ maildir_uidlist_set_ext_real(struct mail
 	rec->extensions = p_malloc(uidlist->record_pool, buf->used);
 	memcpy(rec->extensions, buf->data, buf->used);
 
-	uidlist->recreate = TRUE;
+	if (rec->uid != (uint32_t)-1) {
+		/* message already exists in uidlist, need to recreate it */
+		uidlist->recreate = TRUE;
+	}
 }
 
 void maildir_uidlist_set_ext(struct maildir_uidlist *uidlist, uint32_t uid,
@@ -1206,8 +1214,10 @@ static bool maildir_uidlist_want_compres
 {
 	unsigned int min_rewrite_count;
 
-	if (!ctx->uidlist->initial_read)
+	if (!ctx->uidlist->locked_refresh)
 		return FALSE;
+	if (ctx->uidlist->recreate)
+		return TRUE;
 
 	min_rewrite_count =
 		(ctx->uidlist->read_records_count + ctx->new_files_count) *
@@ -1219,12 +1229,10 @@ static bool maildir_uidlist_want_recreat
 {
 	struct maildir_uidlist *uidlist = ctx->uidlist;
 
-	if (uidlist->recreate ||
-	    ctx->finish_change_counter != uidlist->change_counter)
+	if (!uidlist->locked_refresh)
+		return FALSE;
+	if (ctx->finish_change_counter != uidlist->change_counter)
 		return TRUE;
-
-	if (!uidlist->initial_read)
-		return FALSE;
 
 	if (uidlist->fd == -1 || uidlist->version != UIDLIST_VERSION)
 		return TRUE;
@@ -1284,7 +1292,7 @@ static int maildir_uidlist_sync_update(s
 	if ((uoff_t)st.st_size != file_size) {
 		i_warning("%s: file size changed unexpectedly after write",
 			  uidlist->path);
-	} else {
+	} else if (uidlist->locked_refresh) {
 		uidlist->fd_size = st.st_size;
 		uidlist->last_read_offset = st.st_size;
 		maildir_uidlist_update_hdr(uidlist, &st);
@@ -1494,6 +1502,7 @@ void maildir_uidlist_sync_remove(struct 
 	unsigned int idx;
 
 	i_assert(ctx->partial);
+	i_assert(ctx->uidlist->locked_refresh);
 
 	rec = hash_lookup(ctx->uidlist->files, filename);
 	i_assert(rec != NULL);
@@ -1630,8 +1639,7 @@ void maildir_uidlist_sync_finish(struct 
 	ctx->uidlist->initial_sync = TRUE;
 
 	i_assert(ctx->locked || !ctx->changed);
-	if ((ctx->changed || ctx->uidlist->recreate ||
-	     maildir_uidlist_want_compress(ctx)) &&
+	if ((ctx->changed || maildir_uidlist_want_compress(ctx)) &&
 	    !ctx->failed && ctx->locked) T_BEGIN {
 		if (maildir_uidlist_sync_update(ctx) < 0)
 			ctx->failed = TRUE;