From: Timo Sirainen <tss@iki.fi>
Date: Fri, 20 Mar 2009 22:22:59 +0000 (-0400)
Subject: dbox bugfixes.
X-Git-Tag: 2.0.alpha1~1038^2~32
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=63288a7379b0f4c47f6a8c7cd6d27b7ee7614cc8;p=thirdparty%2Fdovecot%2Fcore.git

dbox bugfixes.

--HG--
branch : HEAD
---

diff --git a/src/lib-storage/index/dbox/dbox-file.c b/src/lib-storage/index/dbox/dbox-file.c
index dab9049c66..7e5f02e1de 100644
--- a/src/lib-storage/index/dbox/dbox-file.c
+++ b/src/lib-storage/index/dbox/dbox-file.c
@@ -612,6 +612,7 @@ dbox_file_seek_next_at_metadata(struct dbox_file *file, uoff_t *offset)
 	const char *line;
 	int ret;
 
+	i_stream_seek(file->input, *offset);
 	if ((ret = dbox_file_metadata_skip_header(file)) <= 0)
 		return ret;
 
@@ -636,22 +637,20 @@ int dbox_file_seek_next(struct dbox_file *file, uoff_t *offset_r, bool *last_r)
 		/* first mail */
 		offset = file->file_header_size;
 	} else {
-		offset = file->cur_offset + file->cur_physical_size;
+		offset = file->cur_offset + file->msg_header_size +
+			file->cur_physical_size;
 		if ((ret = dbox_file_seek_next_at_metadata(file, &offset)) <= 0)
 			return ret;
 	}
 	*offset_r = offset;
-	*last_r = FALSE;
-
-	ret = dbox_file_get_mail_stream(file, offset, &size, NULL, &expunged);
-	if (ret <= 0)
-		return ret;
 
-	if (expunged) {
+	if (file->input != NULL && i_stream_is_eof(file->input)) {
 		*last_r = TRUE;
 		return 0;
 	}
-	return 1;
+	*last_r = FALSE;
+
+	return dbox_file_get_mail_stream(file, offset, &size, NULL, &expunged);
 }
 
 static int
diff --git a/src/lib-storage/index/dbox/dbox-mail.c b/src/lib-storage/index/dbox/dbox-mail.c
index 984fbae7df..252fe2aa79 100644
--- a/src/lib-storage/index/dbox/dbox-mail.c
+++ b/src/lib-storage/index/dbox/dbox-mail.c
@@ -69,14 +69,28 @@ static int dbox_mail_open(struct dbox_mail *mail,
 		if (map_uid == 0) {
 			mail->open_file =
 				dbox_file_init_single(mbox, _mail->uid);
-		} else {
-			ret = dbox_map_lookup(mbox->storage->map, map_uid,
-					      &file_id, &mail->offset);
-			if (ret <= 0) {
-				if (ret == 0)
-					mail_set_expunged(_mail);
+		} else if ((ret = dbox_map_lookup(mbox->storage->map,
+						  map_uid, &file_id,
+						  &mail->offset)) <= 0) {
+			if (ret < 0)
+				return -1;
+
+			/* map_uid doesn't exist anymore. either it
+			   got just expunged or the map index is
+			   corrupted. see if the message still exists
+			   in the mailbox index. */
+			(void)mail_index_refresh(mbox->ibox.index);
+			if (mail_index_is_expunged(mbox->ibox.view,
+						   _mail->seq)) {
+				mail_set_expunged(_mail);
 				return -1;
 			}
+
+			dbox_map_set_corrupted(mbox->storage->map,
+				"Unexpectedly lost map_uid=%u", map_uid);
+			mbox->storage->sync_rebuild = TRUE;
+			return -1;
+		} else {
 			mail->open_file =
 				dbox_file_init_multi(mbox->storage, file_id);
 		}
diff --git a/src/lib-storage/index/dbox/dbox-map-private.h b/src/lib-storage/index/dbox/dbox-map-private.h
index 39cf86433a..f95f3cc0da 100644
--- a/src/lib-storage/index/dbox/dbox-map-private.h
+++ b/src/lib-storage/index/dbox/dbox-map-private.h
@@ -43,6 +43,7 @@ struct dbox_map_append_context {
 	unsigned int committed:1;
 };
 
+int dbox_map_open(struct dbox_map *map);
 int dbox_map_refresh(struct dbox_map *map);
 int dbox_map_view_lookup_rec(struct dbox_map *map, struct mail_index_view *view,
 			     uint32_t seq, struct dbox_mail_lookup_rec *rec_r);
diff --git a/src/lib-storage/index/dbox/dbox-map.c b/src/lib-storage/index/dbox/dbox-map.c
index 68dd2d1b42..af7d7a8434 100644
--- a/src/lib-storage/index/dbox/dbox-map.c
+++ b/src/lib-storage/index/dbox/dbox-map.c
@@ -58,7 +58,7 @@ void dbox_map_deinit(struct dbox_map **_map)
 	i_free(map);
 }
 
-static int dbox_map_open(struct dbox_map *map)
+int dbox_map_open(struct dbox_map *map)
 {
 	struct mail_storage *storage = &map->storage->storage;
 	enum mail_index_open_flags open_flags;
@@ -265,6 +265,24 @@ dbox_map_transaction_begin(struct dbox_map *map)
 	return ctx;
 }
 
+static void
+dbox_map_sync_handle(struct dbox_map *map, struct mail_index_sync_ctx *sync_ctx)
+{
+	struct mail_index_sync_rec sync_rec;
+	uint32_t seq1, seq2;
+	uoff_t offset1, offset2;
+
+	mail_index_sync_get_offsets(sync_ctx, &seq1, &offset1, &seq2, &offset2);
+	if (offset1 != offset2 || seq1 != seq2) {
+		/* something had crashed. need a full resync. */
+		i_warning("dbox %s: Inconsistency in map index",
+			  map->storage->storage_dir);
+		map->storage->sync_rebuild = TRUE;
+	} else {
+		while (mail_index_sync_next(sync_ctx, &sync_rec)) ;
+	}
+}
+
 int dbox_map_transaction_commit(struct dbox_map_transaction_context **_ctx)
 {
 	struct dbox_map_transaction_context *ctx = *_ctx;
@@ -272,17 +290,15 @@ int dbox_map_transaction_commit(struct dbox_map_transaction_context **_ctx)
 	struct mail_index_sync_ctx *sync_ctx;
 	struct mail_index_view *view;
 	struct mail_index_transaction *sync_trans, *trans = ctx->trans;
-	uint32_t seq1, seq2;
-	uoff_t offset1, offset2;
 	int ret;
 
 	*_ctx = NULL;
-	i_free(ctx);
-
 	if (!ctx->changed) {
 		mail_index_transaction_rollback(&trans);
+		i_free(ctx);
 		return 0;
 	}
+	i_free(ctx);
 
 	/* use syncing to lock the transaction log, so that we always see
 	   log's head_offset = tail_offset */
@@ -295,14 +311,7 @@ int dbox_map_transaction_commit(struct dbox_map_transaction_context **_ctx)
 		mail_index_transaction_rollback(&trans);
 		return -1;
 	}
-
-	mail_index_sync_get_offsets(sync_ctx, &seq1, &offset1, &seq2, &offset2);
-	if (offset1 != offset2 || seq1 != seq2) {
-		/* something had crashed. need a full resync. */
-		i_warning("dbox %s: Inconsistency in map index",
-			  map->storage->storage_dir);
-		map->storage->sync_rebuild = TRUE;
-	}
+	dbox_map_sync_handle(map, sync_ctx);
 
 	if (mail_index_transaction_commit(&trans) < 0 ||
 	    mail_index_sync_commit(&sync_ctx) < 0) {
@@ -351,37 +360,45 @@ int dbox_map_update_refcounts(struct dbox_map_transaction_context *ctx,
 	return 0;
 }
 
-int dbox_map_remove_file_id(struct dbox_map_transaction_context *ctx,
-			    uint32_t file_id)
+int dbox_map_remove_file_id(struct dbox_map *map, uint32_t file_id)
 {
-	struct dbox_map *map = ctx->map;
+	struct dbox_map_transaction_context *map_trans;
 	const struct mail_index_header *hdr;
 	const struct dbox_mail_index_map_record *rec;
 	const void *data;
 	bool expunged;
 	uint32_t seq;
+	int ret = 0;
 
 	/* make sure the map is refreshed, otherwise we might be expunging
 	   messages that have already been moved to other files. */
 	if (dbox_map_refresh(map) < 0)
 		return -1;
 
+	/* we need a per-file transaction, otherwise we can't refresh the map */
+	map_trans = dbox_map_transaction_begin(map);
+
 	hdr = mail_index_get_header(map->view);
 	for (seq = 1; seq <= hdr->messages_count; seq++) {
 		mail_index_lookup_ext(map->view, seq, map->map_ext_id,
 				      &data, &expunged);
 		if (data == NULL) {
 			dbox_map_set_corrupted(map, "missing map extension");
-			return -1;
+			ret = -1;
+			break;
 		}
 
 		rec = data;
 		if (rec->file_id == file_id) {
-			ctx->changed = TRUE;
-			mail_index_expunge(ctx->trans, seq);
+			map_trans->changed = TRUE;
+			mail_index_expunge(map_trans->trans, seq);
 		}
 	}
-	return 0;
+	if (ret < 0)
+		dbox_map_transaction_rollback(&map_trans);
+	else
+		(void)dbox_map_transaction_commit(&map_trans);
+	return ret;
 }
 
 struct dbox_map_append_context *
@@ -709,8 +726,7 @@ static int dbox_map_assign_file_ids(struct dbox_map_append_context *ctx,
 {
 	struct dbox_file *const *files;
 	unsigned int i, count;
-	uint32_t first_file_id, file_id, seq1, seq2;
-	uoff_t offset1, offset2;
+	uint32_t first_file_id, file_id;
 	int ret;
 
 	/* start the syncing. we'll need it even if there are no file ids to
@@ -723,15 +739,7 @@ static int dbox_map_assign_file_ids(struct dbox_map_append_context *ctx,
 		mail_index_reset_error(ctx->map->index);
 		return -1;
 	}
-
-	mail_index_sync_get_offsets(ctx->sync_ctx, &seq1, &offset1,
-				    &seq2, &offset2);
-	if (offset1 != offset2 || seq1 != seq2) {
-		/* something had crashed. need a full resync. */
-		i_warning("dbox %s: Inconsistency in map index",
-			  ctx->map->storage->storage_dir);
-		ctx->map->storage->sync_rebuild = TRUE;
-	}
+	dbox_map_sync_handle(ctx->map, ctx->sync_ctx);
 
 	if (dbox_map_get_next_file_id(ctx->map, ctx->sync_view, &file_id) < 0) {
 		mail_index_sync_rollback(&ctx->sync_ctx);
diff --git a/src/lib-storage/index/dbox/dbox-map.h b/src/lib-storage/index/dbox/dbox-map.h
index 49af4f26ca..0d9369ac18 100644
--- a/src/lib-storage/index/dbox/dbox-map.h
+++ b/src/lib-storage/index/dbox/dbox-map.h
@@ -45,8 +45,7 @@ void dbox_map_transaction_rollback(struct dbox_map_transaction_context **ctx);
 
 int dbox_map_update_refcounts(struct dbox_map_transaction_context *ctx,
 			      const ARRAY_TYPE(seq_range) *map_uids, int diff);
-int dbox_map_remove_file_id(struct dbox_map_transaction_context *ctx,
-			    uint32_t file_id);
+int dbox_map_remove_file_id(struct dbox_map *map, uint32_t file_id);
 
 /* Return all files containing messages with zero refcount. */
 const ARRAY_TYPE(seq_range) *dbox_map_get_zero_ref_files(struct dbox_map *map);
diff --git a/src/lib-storage/index/dbox/dbox-storage-rebuild.c b/src/lib-storage/index/dbox/dbox-storage-rebuild.c
index d1efd1ee88..207d8ed589 100644
--- a/src/lib-storage/index/dbox/dbox-storage-rebuild.c
+++ b/src/lib-storage/index/dbox/dbox-storage-rebuild.c
@@ -229,7 +229,7 @@ static int rebuild_apply_map(struct dbox_storage_rebuild_context *ctx)
 {
 	struct dbox_map *map = ctx->storage->map;
 	const struct mail_index_header *hdr;
-	struct dbox_rebuild_msg **msgs, *pos;
+	struct dbox_rebuild_msg **msgs, **pos;
 	struct dbox_rebuild_msg search_msg, *search_msgp = &search_msg;
 	struct dbox_mail_lookup_rec rec;
 	uint32_t seq;
@@ -254,8 +254,8 @@ static int rebuild_apply_map(struct dbox_storage_rebuild_context *ctx)
 			/* map record points to non-existing message. */
 			mail_index_expunge(ctx->trans, seq);
 		} else {
-			pos->map_uid = rec.map_uid;
-			pos->seen_zero_ref_in_map = rec.refcount == 0;
+			(*pos)->map_uid = rec.map_uid;
+			(*pos)->seen_zero_ref_in_map = rec.refcount == 0;
 		}
 	}
 	rebuild_add_missing_map_uids(ctx, hdr->next_uid);
@@ -271,13 +271,14 @@ rebuild_lookup_map_uid(struct dbox_storage_rebuild_context *ctx,
 		       uint32_t map_uid)
 {
 	struct dbox_rebuild_msg search_msg, *search_msgp = &search_msg;
-	struct dbox_rebuild_msg *const *msgs;
+	struct dbox_rebuild_msg *const *msgs, **pos;
 	unsigned int count;
 
 	search_msg.map_uid = map_uid;
 	msgs = array_get(&ctx->msgs, &count);
-	return bsearch(&search_msgp, msgs, count, sizeof(*msgs),
-		       dbox_rebuild_msg_uid_cmp);
+	pos = bsearch(&search_msgp, msgs, count, sizeof(*msgs),
+		      dbox_rebuild_msg_uid_cmp);
+	return pos == NULL ? NULL : *pos;
 }
 
 static void
@@ -334,7 +335,7 @@ rebuild_mailbox_multi(struct dbox_storage_rebuild_context *ctx,
 			/* everything was ok */
 		}
 
-		if (rec != NULL) {
+		if (rec != NULL) T_BEGIN {
 			/* keep this message */
 			rec->refcount++;
 
@@ -344,10 +345,10 @@ rebuild_mailbox_multi(struct dbox_storage_rebuild_context *ctx,
 							 NULL, new_seq, uid);
 
 			new_dbox_rec.map_uid = rec->map_uid;
-			mail_index_update_ext(ctx->trans, new_seq,
+			mail_index_update_ext(trans, new_seq,
 					      mbox->dbox_ext_id,
 					      &new_dbox_rec, NULL);
-		}
+		} T_END;
 	}
 }
 
@@ -411,7 +412,10 @@ static int rebuild_mailboxes(struct dbox_storage_rebuild_context *ctx)
 	while ((info = mailbox_list_iter_next(iter)) != NULL) {
 		if ((info->flags & (MAILBOX_NONEXISTENT |
 				    MAILBOX_NOSELECT)) == 0) {
-			if (rebuild_mailbox(ctx, info->name) < 0) {
+			T_BEGIN {
+				ret = rebuild_mailbox(ctx, info->name);
+			} T_END;
+			if (ret < 0) {
 				ret = -1;
 				break;
 			}
@@ -621,6 +625,9 @@ static int dbox_storage_rebuild_scan(struct dbox_storage_rebuild_context *ctx)
 	unsigned int dir_len;
 	int ret = 0;
 
+	if (dbox_map_open(ctx->storage->map) < 0)
+		return -1;
+
 	/* begin by locking the map, so that other processes can't try to
 	   rebuild at the same time. */
 	ret = mail_index_sync_begin(ctx->storage->map->index, &ctx->sync_ctx,
@@ -649,7 +656,7 @@ static int dbox_storage_rebuild_scan(struct dbox_storage_rebuild_context *ctx)
 			str_truncate(path, dir_len);
 			str_append(path, d->d_name);
 			T_BEGIN {
-				ret = rebuild_add_file(ctx, d->d_name);
+				ret = rebuild_add_file(ctx, str_c(path));
 			} T_END;
 			if (ret < 0) {
 				ret = -1;
diff --git a/src/lib-storage/index/dbox/dbox-storage.c b/src/lib-storage/index/dbox/dbox-storage.c
index c92423a1eb..bc017e2001 100644
--- a/src/lib-storage/index/dbox/dbox-storage.c
+++ b/src/lib-storage/index/dbox/dbox-storage.c
@@ -12,6 +12,7 @@
 #include "dbox-map.h"
 #include "dbox-file.h"
 #include "dbox-sync.h"
+#include "dbox-storage-rebuild.h"
 #include "dbox-storage.h"
 
 #include <stdio.h>
@@ -202,6 +203,11 @@ static void dbox_destroy(struct mail_storage *_storage)
 {
 	struct dbox_storage *storage = (struct dbox_storage *)_storage;
 
+	if (storage->sync_rebuild) {
+		if (dbox_storage_rebuild(storage) < 0)
+			return;
+	}
+
 	dbox_sync_cleanup(storage);
 	dbox_files_free(storage);
 	dbox_map_deinit(&storage->map);
diff --git a/src/lib-storage/index/dbox/dbox-sync-rebuild.c b/src/lib-storage/index/dbox/dbox-sync-rebuild.c
index bf3925c638..e39d306eba 100644
--- a/src/lib-storage/index/dbox/dbox-sync-rebuild.c
+++ b/src/lib-storage/index/dbox/dbox-sync-rebuild.c
@@ -181,7 +181,9 @@ static int dbox_sync_add_file_index(struct dbox_sync_rebuild_context *ctx,
 	}
 
 	mail_index_append(ctx->trans, file->uid, &seq);
-	dbox_sync_rebuild_index_metadata(ctx, file, seq, file->uid);
+	T_BEGIN {
+		dbox_sync_rebuild_index_metadata(ctx, file, seq, file->uid);
+	} T_END;
 	return 0;
 }
 
@@ -406,7 +408,7 @@ dbox_sync_index_rebuild_init(struct dbox_mailbox *mbox,
 #endif
 		open_flags |= MAIL_INDEX_OPEN_FLAG_MMAP_DISABLE;
 	if (mail_index_open(ctx->backup_index, open_flags,
-			    box->storage->lock_method) < 0)
+			    box->storage->lock_method) <= 0)
 		mail_index_free(&ctx->backup_index);
 	else
 		ctx->backup_view = mail_index_view_open(ctx->backup_index);
@@ -477,6 +479,6 @@ int dbox_sync_index_rebuild(struct dbox_mailbox *mbox)
 	mail_index_view_close(&view);
 
 	if (ret == 0)
-		ctx->mbox->storage->sync_rebuild = FALSE;
+		mbox->storage->sync_rebuild = FALSE;
 	return ret;
 }
diff --git a/src/lib-storage/index/dbox/dbox-sync.c b/src/lib-storage/index/dbox/dbox-sync.c
index 808669c278..3433152eb0 100644
--- a/src/lib-storage/index/dbox/dbox-sync.c
+++ b/src/lib-storage/index/dbox/dbox-sync.c
@@ -136,6 +136,7 @@ static int dbox_sync_index(struct dbox_sync_context *ctx)
 	hdr = mail_index_get_header(ctx->sync_view);
 	if (hdr->uid_validity == 0) {
 		/* newly created index file */
+		i_warning("FIXME: newly created");
 		return 0;
 	}
 
@@ -152,9 +153,7 @@ static int dbox_sync_index(struct dbox_sync_context *ctx)
 				       dbox_sync_file_entry_hash,
 				       dbox_sync_file_entry_cmp);
 
-	for (;;) {
-		if (!mail_index_sync_next(ctx->index_sync_ctx, &sync_rec))
-			break;
+	while (mail_index_sync_next(ctx->index_sync_ctx, &sync_rec)) {
 		if ((ret = dbox_sync_add(ctx, &sync_rec)) <= 0)
 			break;
 	}
@@ -171,8 +170,10 @@ static int dbox_sync_index(struct dbox_sync_context *ctx)
 		hash_table_iterate_deinit(&iter);
 	}
 
-	if (ret == 0)
-		ret = dbox_map_transaction_commit(&ctx->map_trans);
+	if (ret > 0 && ctx->map_trans != NULL) {
+		if (dbox_map_transaction_commit(&ctx->map_trans) < 0)
+			ret = -1;
+	}
 
 	if (box->v.sync_notify != NULL)
 		box->v.sync_notify(box, 0, 0);
@@ -196,7 +197,8 @@ static int dbox_refresh_header(struct dbox_mailbox *mbox)
 		if (data_size != 0 && data_size != 4) {
 			i_warning("dbox %s: Invalid dbox header size",
 				  mbox->path);
-		}
+		} else
+			i_warning("FIXME: initial sync");
 		return -1;
 	}
 	hdr = data;
@@ -259,6 +261,7 @@ int dbox_sync_begin(struct dbox_mailbox *mbox, enum dbox_sync_flags flags,
 				return dbox_sync_begin(mbox, flags, ctx_r);
 			}
 			ret = 0;
+			i_warning("FIXME: poop");
 		} else {
 			if ((ret = dbox_sync_index(ctx)) > 0)
 				break;
@@ -274,6 +277,8 @@ int dbox_sync_begin(struct dbox_mailbox *mbox, enum dbox_sync_flags flags,
 				ret = -1;
 			} else {
 				/* do a full resync and try again. */
+				i_warning("dbox %s: Rebuilding index",
+					  ctx->mbox->path);
 				ret = dbox_sync_index_rebuild(mbox);
 			}
 		}
@@ -343,15 +348,12 @@ dbox_storage_sync_init(struct mailbox *box, enum mailbox_sync_flags flags)
 void dbox_sync_cleanup(struct dbox_storage *storage)
 {
 	const ARRAY_TYPE(seq_range) *ref0_file_ids;
-	struct dbox_map_transaction_context *map_trans;
 	struct dbox_file *file;
 	struct seq_range_iter iter;
 	unsigned int i = 0;
 	uint32_t file_id;
 	bool deleted;
 
-	map_trans = dbox_map_transaction_begin(storage->map);
-
 	ref0_file_ids = dbox_map_get_zero_ref_files(storage->map);
 	seq_range_array_iter_init(&iter, ref0_file_ids); i = 0;
 	while (seq_range_array_iter_nth(&iter, i++, &file_id)) T_BEGIN {
@@ -359,9 +361,7 @@ void dbox_sync_cleanup(struct dbox_storage *storage)
 		if (dbox_file_open_or_create(file, &deleted) > 0 && !deleted)
 			(void)dbox_sync_file_cleanup(file);
 		else
-			dbox_map_remove_file_id(map_trans, file_id);
+			dbox_map_remove_file_id(storage->map, file_id);
 		dbox_file_unref(&file);
 	} T_END;
-
-	(void)dbox_map_transaction_commit(&map_trans);
 }