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

[dovecot at dovecot.org]

details:   http://hg.dovecot.org/dovecot-2.0/rev/9b6ca1680cd2
changeset: 9206:9b6ca1680cd2
user:      Timo Sirainen <tss at iki.fi>
date:      Mon May 04 16:38:38 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 1cdee5c559ca -r 9b6ca1680cd2 src/lib-storage/index/maildir/maildir-uidlist.c
--- a/src/lib-storage/index/maildir/maildir-uidlist.c	Mon May 04 12:32:59 2009 -0400
+++ b/src/lib-storage/index/maildir/maildir-uidlist.c	Mon May 04 16:38:38 2009 -0400
@@ -93,6 +93,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 {
@@ -173,6 +174,7 @@ static int maildir_uidlist_lock_timeout(
 	}
 
 	uidlist->lock_count++;
+	uidlist->locked_refresh = FALSE;
 
 	if (refresh) {
 		/* make sure we have the latest changes before
@@ -214,6 +216,7 @@ void maildir_uidlist_unlock(struct maild
 	if (--uidlist->lock_count > 0)
 		return;
 
+	uidlist->locked_refresh = FALSE;
 	(void)file_dotlock_delete(&uidlist->dotlock);
 }
 
@@ -815,6 +818,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;
 }
@@ -1022,7 +1027,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,
@@ -1213,8 +1221,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) *
@@ -1226,12 +1236,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;
@@ -1291,7 +1299,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);
@@ -1530,6 +1538,7 @@ void maildir_uidlist_sync_remove(struct 
 	unsigned int idx;
 
 	i_assert(ctx->partial);
+	i_assert(ctx->uidlist->locked_refresh);
 
 	rec = hash_table_lookup(ctx->uidlist->files, filename);
 	i_assert(rec != NULL);
@@ -1673,8 +1682,7 @@ void maildir_uidlist_sync_finish(struct 
 	/* mbox=NULL means we're coming from dbox rebuilding code.
 	   the dbox is already locked, so allow uidlist recreation */
 	i_assert(ctx->locked || !ctx->changed || ctx->uidlist->mbox == NULL);
-	if ((ctx->changed || ctx->uidlist->recreate ||
-	     maildir_uidlist_want_compress(ctx)) &&
+	if ((ctx->changed || maildir_uidlist_want_compress(ctx)) &&
 	    !ctx->failed && (ctx->locked || ctx->uidlist->mbox == NULL)) {
 		T_BEGIN {
 			if (maildir_uidlist_sync_update(ctx) < 0)