dovecot-2.0: Maildir: More fixes to uidlist handling.

dovecot at dovecot.org dovecot at dovecot.org
Mon May 4 23:48:46 EEST 2009


details:   http://hg.dovecot.org/dovecot-2.0/rev/236509dddd3c
changeset: 9208:236509dddd3c
user:      Timo Sirainen <tss at iki.fi>
date:      Mon May 04 16:45:28 2009 -0400
description:
Maildir: More fixes to uidlist handling.

diffstat:

5 files changed, 100 insertions(+), 24 deletions(-)
src/lib-storage/index/maildir/maildir-save.c       |    7 -
src/lib-storage/index/maildir/maildir-sync-index.c |    3 
src/lib-storage/index/maildir/maildir-sync.c       |    4 
src/lib-storage/index/maildir/maildir-uidlist.c    |  107 ++++++++++++++++----
src/lib-storage/index/maildir/maildir-uidlist.h    |    3 

diffs (truncated from 340 to 300 lines):

diff -r d04b97ed7ac4 -r 236509dddd3c src/lib-storage/index/maildir/maildir-save.c
--- a/src/lib-storage/index/maildir/maildir-save.c	Mon May 04 16:40:41 2009 -0400
+++ b/src/lib-storage/index/maildir/maildir-save.c	Mon May 04 16:45:28 2009 -0400
@@ -690,7 +690,7 @@ int maildir_transaction_save_commit_pre(
 		return -1;
 	}
 
-	ctx->locked = ret > 0;
+	ctx->locked = maildir_uidlist_is_locked(ctx->mbox->uidlist);
 	if (ctx->locked) {
 		if (maildir_transaction_save_commit_pre_sync(ctx) < 0) {
 			maildir_transaction_save_rollback(ctx);
@@ -760,7 +760,8 @@ int maildir_transaction_save_commit_pre(
 
 	if (ctx->uidlist_sync_ctx != NULL) {
 		/* update dovecot-uidlist file. */
-		if (maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx) < 0)
+		if (maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx,
+						ret >= 0) < 0)
 			ret = -1;
 	}
 
@@ -843,7 +844,7 @@ maildir_transaction_save_rollback_real(s
 		maildir_transaction_unlink_copied_files(ctx, NULL);
 
 	if (ctx->uidlist_sync_ctx != NULL)
-		(void)maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx);
+		(void)maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx, FALSE);
 	if (ctx->locked)
 		maildir_uidlist_unlock(ctx->mbox->uidlist);
 	if (ctx->sync_ctx != NULL)
diff -r d04b97ed7ac4 -r 236509dddd3c src/lib-storage/index/maildir/maildir-sync-index.c
--- a/src/lib-storage/index/maildir/maildir-sync-index.c	Mon May 04 16:40:41 2009 -0400
+++ b/src/lib-storage/index/maildir/maildir-sync-index.c	Mon May 04 16:45:28 2009 -0400
@@ -556,7 +556,8 @@ int maildir_sync_index(struct maildir_in
 	mail_index_view_close(&view2);
 
 	if (ctx->uidlist_sync_ctx != NULL) {
-		if (maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx) < 0)
+		if (maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx,
+						TRUE) < 0)
 			ret = -1;
 	}
 
diff -r d04b97ed7ac4 -r 236509dddd3c src/lib-storage/index/maildir/maildir-sync.c
--- a/src/lib-storage/index/maildir/maildir-sync.c	Mon May 04 16:40:41 2009 -0400
+++ b/src/lib-storage/index/maildir/maildir-sync.c	Mon May 04 16:45:28 2009 -0400
@@ -266,7 +266,7 @@ static void maildir_sync_deinit(struct m
 static void maildir_sync_deinit(struct maildir_sync_context *ctx)
 {
 	if (ctx->uidlist_sync_ctx != NULL)
-		(void)maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx);
+		(void)maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx, FALSE);
 	if (ctx->index_sync_ctx != NULL) {
 		(void)maildir_sync_index_finish(&ctx->index_sync_ctx,
 						TRUE, FALSE);
@@ -868,7 +868,7 @@ static int maildir_sync_context(struct m
 		}
 	}
 
-	return maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx);
+	return maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx, TRUE);
 }
 
 int maildir_storage_sync_force(struct maildir_mailbox *mbox, uint32_t uid)
diff -r d04b97ed7ac4 -r 236509dddd3c src/lib-storage/index/maildir/maildir-uidlist.c
--- a/src/lib-storage/index/maildir/maildir-uidlist.c	Mon May 04 16:40:41 2009 -0400
+++ b/src/lib-storage/index/maildir/maildir-uidlist.c	Mon May 04 16:45:28 2009 -0400
@@ -91,9 +91,9 @@ struct maildir_uidlist {
 	unsigned int recreate:1;
 	unsigned int initial_read:1;
 	unsigned int initial_hdr_read:1;
-	unsigned int initial_sync:1;
 	unsigned int retry_rewind:1;
 	unsigned int locked_refresh:1;
+	unsigned int unsorted:1;
 };
 
 struct maildir_uidlist_sync_ctx {
@@ -127,7 +127,8 @@ static bool maildir_uidlist_iter_next_re
 					  struct maildir_uidlist_rec **rec_r);
 
 static int maildir_uidlist_lock_timeout(struct maildir_uidlist *uidlist,
-					bool nonblock, bool refresh)
+					bool nonblock, bool refresh,
+					bool refresh_when_locked)
 {
 	struct mailbox *box = &uidlist->ibox->box;
 	const char *control_dir, *path;
@@ -137,6 +138,10 @@ static int maildir_uidlist_lock_timeout(
 	int i, ret;
 
 	if (uidlist->lock_count > 0) {
+		if (!uidlist->locked_refresh && refresh_when_locked) {
+			if (maildir_uidlist_refresh(uidlist) < 0)
+				return -1;
+		}
 		uidlist->lock_count++;
 		return 1;
 	}
@@ -189,12 +194,12 @@ static int maildir_uidlist_lock_timeout(
 
 int maildir_uidlist_lock(struct maildir_uidlist *uidlist)
 {
-	return maildir_uidlist_lock_timeout(uidlist, FALSE, TRUE);
+	return maildir_uidlist_lock_timeout(uidlist, FALSE, TRUE, FALSE);
 }
 
 int maildir_uidlist_try_lock(struct maildir_uidlist *uidlist)
 {
-	return maildir_uidlist_lock_timeout(uidlist, TRUE, TRUE);
+	return maildir_uidlist_lock_timeout(uidlist, TRUE, TRUE, FALSE);
 }
 
 int maildir_uidlist_lock_touch(struct maildir_uidlist *uidlist)
@@ -293,6 +298,16 @@ static void maildir_uidlist_close(struct
 	uidlist->read_line_count = 0;
 }
 
+static void maildir_uidlist_reset(struct maildir_uidlist *uidlist)
+{
+	maildir_uidlist_close(uidlist);
+	uidlist->last_seen_uid = 0;
+	uidlist->initial_hdr_read = FALSE;
+
+	hash_table_clear(uidlist->files, FALSE);
+	array_clear(&uidlist->records);
+}
+
 void maildir_uidlist_deinit(struct maildir_uidlist **_uidlist)
 {
 	struct maildir_uidlist *uidlist = *_uidlist;
@@ -422,7 +437,8 @@ static bool maildir_uidlist_next(struct 
 static bool maildir_uidlist_next(struct maildir_uidlist *uidlist,
 				 const char *line)
 {
-	struct maildir_uidlist_rec *rec, *old_rec;
+	struct maildir_uidlist_rec *rec, *old_rec, *const *recs;
+	unsigned int count;
 	uint32_t uid;
 
 	uid = 0;
@@ -487,7 +503,25 @@ static bool maildir_uidlist_next(struct 
 	}
 
 	old_rec = hash_table_lookup(uidlist->files, line);
-	if (old_rec != NULL) {
+	if (old_rec == NULL) {
+		/* no conflicts */
+	} else if (old_rec->uid == uid) {
+		/* most likely this is a record we saved ourself, but couldn't
+		   update last_seen_uid because uidlist wasn't refreshed while
+		   it was locked.
+
+		   another possibility is a duplicate file record. currently
+		   it would be a bug, but not that big of a deal. also perhaps
+		   in future such duplicate lines could be used to update
+		   extended fields. so just let it through anyway.
+
+		   we'll waste a bit of memory here by allocating the record
+		   twice, but that's not really a problem.  */
+		rec->filename = old_rec->filename;
+		hash_table_insert(uidlist->files, rec->filename, rec);
+		uidlist->unsorted = TRUE;
+		return TRUE;
+	} else {
 		/* This can happen if expunged file is moved back and the file
 		   was appended to uidlist. */
 		i_warning("%s: Duplicate file entry at line %u: "
@@ -500,6 +534,13 @@ static bool maildir_uidlist_next(struct 
 		*old_rec = *rec;
 		rec = old_rec;
 		uidlist->recreate = TRUE;
+	}
+
+	recs = array_get(&uidlist->records, &count);
+	if (count > 0 && recs[count-1]->uid > uid) {
+		/* we most likely have some records in the array that we saved
+		   ourself without refreshing uidlist */
+		uidlist->unsorted = TRUE;
 	}
 
 	rec->filename = p_strdup(uidlist->record_pool, line);
@@ -591,6 +632,17 @@ static int maildir_uidlist_read_header(s
 	uidlist->uid_validity = uid_validity;
 	uidlist->next_uid = next_uid;
 	return 1;
+}
+
+static void maildir_uidlist_records_sort_by_uid(struct maildir_uidlist *uidlist)
+{
+	struct maildir_uidlist_rec **recs;
+	unsigned int count;
+
+	recs = array_get_modifiable(&uidlist->records, &count);
+	qsort(recs, count, sizeof(*recs), maildir_uid_cmp);
+
+	uidlist->unsorted = FALSE;
 }
 
 static int
@@ -685,9 +737,13 @@ maildir_uidlist_update_read(struct maild
 		if (input->stream_errno != 0)
                         ret = -1;
 
+		if (uidlist->unsorted) {
+			uidlist->recreate = TRUE;
+			maildir_uidlist_records_sort_by_uid(uidlist);
+		}
 		if (uidlist->next_uid <= uidlist->prev_read_uid)
 			uidlist->next_uid = uidlist->prev_read_uid + 1;
-		if (uidlist->next_uid < orig_next_uid) {
+		if (ret > 0 && uidlist->next_uid < orig_next_uid) {
 			mail_storage_set_critical(storage,
 				"%s: next_uid was lowered (%u -> %u)",
 				uidlist->path, orig_next_uid,
@@ -717,6 +773,7 @@ maildir_uidlist_update_read(struct maild
 			mail_storage_set_critical(storage,
 				"read(%s) failed: %m", uidlist->path);
 		}
+		uidlist->last_read_offset = 0;
 	}
 
 	i_stream_destroy(&input);
@@ -800,8 +857,11 @@ int maildir_uidlist_refresh(struct maild
 
 	if (uidlist->fd != -1) {
 		ret = maildir_uidlist_has_changed(uidlist, &recreated);
-		if (ret <= 0)
+		if (ret <= 0) {
+			if (UIDLIST_IS_LOCKED(uidlist))
+				uidlist->locked_refresh = TRUE;
 			return ret;
+		}
 
 		if (recreated)
 			maildir_uidlist_close(uidlist);
@@ -1248,9 +1308,9 @@ static bool maildir_uidlist_want_recreat
 
 	if (!uidlist->locked_refresh)
 		return FALSE;
+
 	if (ctx->finish_change_counter != uidlist->change_counter)
 		return TRUE;
-
 	if (uidlist->fd == -1 || uidlist->version != UIDLIST_VERSION)
 		return TRUE;
 	return maildir_uidlist_want_compress(ctx);
@@ -1351,7 +1411,7 @@ static int maildir_uidlist_sync_lock(str
 	nonblock = (sync_flags & MAILDIR_UIDLIST_SYNC_TRYLOCK) != 0;
 	refresh = (sync_flags & MAILDIR_UIDLIST_SYNC_NOREFRESH) == 0;
 
-	ret = maildir_uidlist_lock_timeout(uidlist, nonblock, refresh);
+	ret = maildir_uidlist_lock_timeout(uidlist, nonblock, refresh, refresh);
 	if (ret <= 0) {
 		if (ret < 0 || !nonblock)
 			return ret;
@@ -1401,6 +1461,7 @@ int maildir_uidlist_sync_init(struct mai
 		}
 		return 1;
 	}
+	i_assert(uidlist->locked_refresh);
 
 	ctx->record_pool = pool_alloconly_create(MEMPOOL_GROWING
 						 "maildir_uidlist_sync", 16384);
@@ -1632,10 +1693,11 @@ static void maildir_uidlist_assign_uids(
 		recs[dest]->flags &= ~MAILDIR_UIDLIST_REC_FLAG_MOVED;
 	}
 
+	if (ctx->uidlist->locked_refresh)
+		ctx->uidlist->last_seen_uid = ctx->uidlist->next_uid-1;
+
 	ctx->new_files_count = 0;
 	ctx->first_nouid_pos = (unsigned int)-1;
-
-        ctx->uidlist->last_seen_uid = ctx->uidlist->next_uid-1;
 	ctx->uidlist->change_counter++;
 	ctx->finish_change_counter = ctx->uidlist->change_counter;
 }
@@ -1682,12 +1744,14 @@ void maildir_uidlist_sync_finish(struct 
 		if (!ctx->failed)
 			maildir_uidlist_swap(ctx);
 	} else {
-		if (ctx->new_files_count != 0)
+		if (ctx->new_files_count != 0 && !ctx->failed) {
+			i_assert(ctx->changed);
+			i_assert(ctx->locked);
 			maildir_uidlist_assign_uids(ctx);
+		}
 	}
 
 	ctx->finished = TRUE;
-	ctx->uidlist->initial_sync = TRUE;
 
 	/* mbox=NULL means we're coming from dbox rebuilding code.
 	   the dbox is already locked, so allow uidlist recreation */
@@ -1695,18 +1759,27 @@ void maildir_uidlist_sync_finish(struct 
 	if ((ctx->changed || maildir_uidlist_want_compress(ctx)) &&
 	    !ctx->failed && (ctx->locked || ctx->uidlist->mbox == NULL)) {
 		T_BEGIN {


More information about the dovecot-cvs mailing list