Mercurial > dovecot > original-hg > dovecot-1.2

changeset 9018:0bb192fe0abd HEAD

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Maildir: More fixes to uidlist handling.
author Timo Sirainen <tss@iki.fi>
date Mon, 04 May 2009 16:43:59 -0400
parents 0aa17f3e4a6d
children 7ccc533e30bb
files src/lib-storage/index/maildir/maildir-save.c src/lib-storage/index/maildir/maildir-sync-index.c src/lib-storage/index/maildir/maildir-sync.c src/lib-storage/index/maildir/maildir-uidlist.c src/lib-storage/index/maildir/maildir-uidlist.h
diffstat 5 files changed, 100 insertions(+), 24 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-storage/index/maildir/maildir-save.c	Mon May 04 16:42:43 2009 -0400
+++ b/src/lib-storage/index/maildir/maildir-save.c	Mon May 04 16:43:59 2009 -0400
@@ -691,7 +691,7 @@
 		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);
@@ -761,7 +761,8 @@
 
 	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;
 	}
 
@@ -844,7 +845,7 @@
 		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)
--- a/src/lib-storage/index/maildir/maildir-sync-index.c	Mon May 04 16:42:43 2009 -0400
+++ b/src/lib-storage/index/maildir/maildir-sync-index.c	Mon May 04 16:43:59 2009 -0400
@@ -556,7 +556,8 @@
 	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;
 	}
 
--- a/src/lib-storage/index/maildir/maildir-sync.c	Mon May 04 16:42:43 2009 -0400
+++ b/src/lib-storage/index/maildir/maildir-sync.c	Mon May 04 16:43:59 2009 -0400
@@ -266,7 +266,7 @@
 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);
@@ -866,7 +866,7 @@
 		}
 	}
 
-	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)
--- a/src/lib-storage/index/maildir/maildir-uidlist.c	Mon May 04 16:42:43 2009 -0400
+++ b/src/lib-storage/index/maildir/maildir-uidlist.c	Mon May 04 16:43:59 2009 -0400
@@ -89,9 +89,9 @@
 	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 {
@@ -125,7 +125,8 @@
 					  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;
@@ -135,6 +136,10 @@
 	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;
 	}
@@ -187,12 +192,12 @@
 
 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)
@@ -292,6 +297,16 @@
 	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;
@@ -421,7 +436,8 @@
 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;
@@ -486,7 +502,25 @@
 	}
 
 	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: "
@@ -501,6 +535,13 @@
 		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);
 	hash_table_insert(uidlist->files, rec->filename, rec);
 	array_append(&uidlist->records, &rec, 1);
@@ -592,6 +633,17 @@
 	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
 maildir_uidlist_update_read(struct maildir_uidlist *uidlist,
 			    bool *retry_r, bool try_retry)
@@ -684,9 +736,13 @@
 		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,
@@ -716,6 +772,7 @@
 			mail_storage_set_critical(storage,
 				"read(%s) failed: %m", uidlist->path);
 		}
+		uidlist->last_read_offset = 0;
 	}
 
 	i_stream_destroy(&input);
@@ -799,8 +856,11 @@
 
 	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);
@@ -1247,9 +1307,9 @@
 
 	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);
@@ -1350,7 +1410,7 @@
 	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;
@@ -1400,6 +1460,7 @@
 		}
 		return 1;
 	}
+	i_assert(uidlist->locked_refresh);
 
 	ctx->record_pool = pool_alloconly_create(MEMPOOL_GROWING
 						 "maildir_uidlist_sync", 16384);
@@ -1627,10 +1688,11 @@
 		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;
 }
@@ -1672,28 +1734,39 @@
 		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;
 
 	i_assert(ctx->locked || !ctx->changed);
 	if ((ctx->changed || maildir_uidlist_want_compress(ctx)) &&
 	    !ctx->failed && ctx->locked) T_BEGIN {
-		if (maildir_uidlist_sync_update(ctx) < 0)
+		if (maildir_uidlist_sync_update(ctx) < 0) {
+			/* we couldn't write everything we wanted. make sure
+			   we don't continue using those UIDs */
+			maildir_uidlist_reset(ctx->uidlist);
 			ctx->failed = TRUE;
+		}
 	} T_END;
 }
 
-int maildir_uidlist_sync_deinit(struct maildir_uidlist_sync_ctx **_ctx)
+int maildir_uidlist_sync_deinit(struct maildir_uidlist_sync_ctx **_ctx,
+				bool success)
 {
 	struct maildir_uidlist_sync_ctx *ctx = *_ctx;
-	int ret = ctx->failed ? -1 : 0;
+	int ret;
 
 	*_ctx = NULL;
 
+	if (!success)
+		ctx->failed = TRUE;
+	ret = ctx->failed ? -1 : 0;
+
 	if (!ctx->finished)
 		maildir_uidlist_sync_finish(ctx);
 	if (ctx->partial)
--- a/src/lib-storage/index/maildir/maildir-uidlist.h	Mon May 04 16:42:43 2009 -0400
+++ b/src/lib-storage/index/maildir/maildir-uidlist.h	Mon May 04 16:43:59 2009 -0400
@@ -109,7 +109,8 @@
 maildir_uidlist_sync_get_full_filename(struct maildir_uidlist_sync_ctx *ctx,
 				       const char *filename);
 void maildir_uidlist_sync_finish(struct maildir_uidlist_sync_ctx *ctx);
-int maildir_uidlist_sync_deinit(struct maildir_uidlist_sync_ctx **ctx);
+int maildir_uidlist_sync_deinit(struct maildir_uidlist_sync_ctx **ctx,
+				bool success);
 
 bool maildir_uidlist_get_uid(struct maildir_uidlist *uidlist,
 			     const char *filename, uint32_t *uid_r);