dovecot-2.0: dbox, mdbox: Fixed race conditions when creating ma...

dovecot at dovecot.org dovecot at dovecot.org
Fri Jun 4 23:09:21 EEST 2010


details:   http://hg.dovecot.org/dovecot-2.0/rev/7fc5db26f6eb
changeset: 11484:7fc5db26f6eb
user:      Timo Sirainen <tss at iki.fi>
date:      Fri Jun 04 21:09:12 2010 +0100
description:
dbox, mdbox: Fixed race conditions when creating mailboxes.

diffstat:

 src/lib-storage/index/dbox-common/dbox-storage.c         |  23 +++++++-
 src/lib-storage/index/dbox-common/dbox-storage.h         |   3 +-
 src/lib-storage/index/dbox-multi/mdbox-map-private.h     |   1 -
 src/lib-storage/index/dbox-multi/mdbox-map.c             |  58 +++++++++++++++---
 src/lib-storage/index/dbox-multi/mdbox-map.h             |   3 +-
 src/lib-storage/index/dbox-multi/mdbox-storage-rebuild.c |  10 ---
 src/lib-storage/index/dbox-multi/mdbox-storage.c         |  29 ++++++---
 src/lib-storage/index/dbox-multi/mdbox-sync.c            |   4 +-
 src/lib-storage/index/dbox-single/sdbox-storage.c        |  31 ++++++----
 9 files changed, 111 insertions(+), 51 deletions(-)

diffs (truncated from 366 to 300 lines):

diff -r 6b35189988dc -r 7fc5db26f6eb src/lib-storage/index/dbox-common/dbox-storage.c
--- a/src/lib-storage/index/dbox-common/dbox-storage.c	Fri Jun 04 19:57:49 2010 +0100
+++ b/src/lib-storage/index/dbox-common/dbox-storage.c	Fri Jun 04 21:09:12 2010 +0100
@@ -94,6 +94,10 @@
 			const struct mailbox_update *update, bool directory)
 {
 	struct dbox_storage *storage = (struct dbox_storage *)box->storage;
+	struct mail_index_sync_ctx *sync_ctx;
+	struct mail_index_view *view;
+	struct mail_index_transaction *trans;
+	int ret;
 
 	if (directory &&
 	    (box->list->props & MAILBOX_LIST_PROP_NO_NOSELECT) == 0)
@@ -101,7 +105,22 @@
 
 	if (index_storage_mailbox_open(box, FALSE) < 0)
 		return -1;
-	if (storage->v.mailbox_create_indexes(box, update) < 0)
+
+	/* use syncing as a lock */
+	ret = mail_index_sync_begin(box->index, &sync_ctx, &view, &trans, 0);
+	if (ret <= 0) {
+		i_assert(ret != 0);
+		mail_storage_set_internal_error(box->storage);
+		mail_index_reset_error(box->index);
 		return -1;
-	return 0;
+	}
+
+	if (mail_index_get_header(view)->uid_validity == 0) {
+		if (storage->v.mailbox_create_indexes(box, update, trans) < 0) {
+			mail_index_sync_rollback(&sync_ctx);
+			return -1;
+		}
+	}
+
+	return mail_index_sync_commit(&sync_ctx);
 }
diff -r 6b35189988dc -r 7fc5db26f6eb src/lib-storage/index/dbox-common/dbox-storage.h
--- a/src/lib-storage/index/dbox-common/dbox-storage.h	Fri Jun 04 19:57:49 2010 +0100
+++ b/src/lib-storage/index/dbox-common/dbox-storage.h	Fri Jun 04 21:09:12 2010 +0100
@@ -37,7 +37,8 @@
 			 struct dbox_file **file_r);
 	/* create/update mailbox indexes */
 	int (*mailbox_create_indexes)(struct mailbox *box,
-				      const struct mailbox_update *update);
+				      const struct mailbox_update *update,
+				      struct mail_index_transaction *trans);
 	/* mark the file corrupted */
 	void (*set_file_corrupted)(struct dbox_file *file);
 };
diff -r 6b35189988dc -r 7fc5db26f6eb src/lib-storage/index/dbox-multi/mdbox-map-private.h
--- a/src/lib-storage/index/dbox-multi/mdbox-map-private.h	Fri Jun 04 19:57:49 2010 +0100
+++ b/src/lib-storage/index/dbox-multi/mdbox-map-private.h	Fri Jun 04 21:09:12 2010 +0100
@@ -16,7 +16,6 @@
 
 	struct mail_index *index;
 	struct mail_index_view *view;
-	uint32_t created_uid_validity;
 
 	uint32_t map_ext_id, ref_ext_id;
 
diff -r 6b35189988dc -r 7fc5db26f6eb src/lib-storage/index/dbox-multi/mdbox-map.c
--- a/src/lib-storage/index/dbox-multi/mdbox-map.c	Fri Jun 04 19:57:49 2010 +0100
+++ b/src/lib-storage/index/dbox-multi/mdbox-map.c	Fri Jun 04 21:09:12 2010 +0100
@@ -30,6 +30,8 @@
 	unsigned int success:1;
 };
 
+static int mdbox_map_generate_uid_validity(struct mdbox_map *map);
+
 void mdbox_map_set_corrupted(struct mdbox_map *map, const char *format, ...)
 {
 	va_list args;
@@ -62,7 +64,6 @@
 				sizeof(uint32_t));
 	map->ref_ext_id = mail_index_ext_register(map->index, "ref", 0,
 				sizeof(uint16_t), sizeof(uint16_t));
-	map->created_uid_validity = ioloop_time;
 
 	mailbox_list_get_permissions(root_list, NULL, &map->create_mode,
 				     &map->create_gid, &map->create_gid_origin);
@@ -146,6 +147,15 @@
 
 	map->view = mail_index_view_open(map->index);
 	mdbox_map_cleanup(map);
+
+	if (mail_index_get_header(map->view)->uid_validity == 0) {
+		if (mdbox_map_generate_uid_validity(map) < 0) {
+			mail_storage_set_internal_error(MAP_STORAGE(map));
+			mail_index_reset_error(map->index);
+			mail_index_close(map->index);
+			return -1;
+		}
+	}
 	return 1;
 }
 
@@ -1218,19 +1228,45 @@
 	i_free(ctx);
 }
 
+static int mdbox_map_generate_uid_validity(struct mdbox_map *map)
+{
+	const struct mail_index_header *hdr;
+	struct mail_index_sync_ctx *sync_ctx;
+	struct mail_index_view *view;
+	struct mail_index_transaction *trans;
+	uint32_t uid_validity;
+	int ret;
+
+	/* do this inside syncing, so that we're locked and there are no
+	   race conditions */
+	ret = mail_index_sync_begin(map->index, &sync_ctx, &view, &trans, 0);
+	if (ret <= 0) {
+		i_assert(ret != 0);
+		return -1;
+	}
+	mdbox_map_sync_handle(map, sync_ctx);
+
+	hdr = mail_index_get_header(map->view);
+	if (hdr->uid_validity != 0) {
+		/* someone else beat us to it */
+		uid_validity = hdr->uid_validity;
+	} else {
+		uid_validity = ioloop_time;
+		mail_index_update_header(trans,
+			offsetof(struct mail_index_header, uid_validity),
+			&uid_validity, sizeof(uid_validity), TRUE);
+	}
+	return mail_index_sync_commit(&sync_ctx);
+}
+
 uint32_t mdbox_map_get_uid_validity(struct mdbox_map *map)
 {
-	const struct mail_index_header *hdr;
+	uint32_t uid_validity;
 
 	i_assert(map->view != NULL);
 
-	hdr = mail_index_get_header(map->view);
-	if (hdr->uid_validity != 0)
-		return hdr->uid_validity;
-
-	/* refresh index in case it was just changed */
-	(void)mdbox_map_refresh(map);
-	hdr = mail_index_get_header(map->view);
-	return hdr->uid_validity != 0 ? hdr->uid_validity :
-		map->created_uid_validity;
+	uid_validity = mail_index_get_header(map->view)->uid_validity;
+	if (uid_validity == 0)
+		mdbox_map_set_corrupted(map, "lost uidvalidity");
+	return uid_validity;
 }
diff -r 6b35189988dc -r 7fc5db26f6eb src/lib-storage/index/dbox-multi/mdbox-map.h
--- a/src/lib-storage/index/dbox-multi/mdbox-map.h	Fri Jun 04 19:57:49 2010 +0100
+++ b/src/lib-storage/index/dbox-multi/mdbox-map.h	Fri Jun 04 21:09:12 2010 +0100
@@ -101,8 +101,7 @@
 int mdbox_map_append_commit(struct mdbox_map_append_context *ctx);
 void mdbox_map_append_free(struct mdbox_map_append_context **ctx);
 
-/* Get either existing uidvalidity or create a new one if map was
-   just created. */
+/* Returns map's uidvalidity */
 uint32_t mdbox_map_get_uid_validity(struct mdbox_map *map);
 
 void mdbox_map_set_corrupted(struct mdbox_map *map, const char *format, ...)
diff -r 6b35189988dc -r 7fc5db26f6eb src/lib-storage/index/dbox-multi/mdbox-storage-rebuild.c
--- a/src/lib-storage/index/dbox-multi/mdbox-storage-rebuild.c	Fri Jun 04 19:57:49 2010 +0100
+++ b/src/lib-storage/index/dbox-multi/mdbox-storage-rebuild.c	Fri Jun 04 21:09:12 2010 +0100
@@ -812,8 +812,6 @@
 
 static int mdbox_storage_rebuild_scan(struct mdbox_storage_rebuild_context *ctx)
 {
-	const struct mail_index_header *hdr;
-	uint32_t uid_validity;
 	int ret = 0;
 
 	if (mdbox_map_open_or_create(ctx->storage->map) < 0)
@@ -840,14 +838,6 @@
 
 	i_warning("mdbox %s: rebuilding indexes", ctx->storage->storage_dir);
 
-	uid_validity = mdbox_map_get_uid_validity(ctx->storage->map);
-	hdr = mail_index_get_header(ctx->sync_view);
-	if (hdr->uid_validity != uid_validity) {
-		mail_index_update_header(ctx->trans,
-			offsetof(struct mail_index_header, uid_validity),
-			&uid_validity, sizeof(uid_validity), TRUE);
-	}
-
 	if (mdbox_storage_rebuild_scan_dir(ctx, ctx->storage->storage_dir,
 					   FALSE) < 0)
 		return -1;
diff -r 6b35189988dc -r 7fc5db26f6eb src/lib-storage/index/dbox-multi/mdbox-storage.c
--- a/src/lib-storage/index/dbox-multi/mdbox-storage.c	Fri Jun 04 19:57:49 2010 +0100
+++ b/src/lib-storage/index/dbox-multi/mdbox-storage.c	Fri Jun 04 21:09:12 2010 +0100
@@ -182,19 +182,23 @@
 }
 
 static int mdbox_write_index_header(struct mailbox *box,
-				    const struct mailbox_update *update)
+				    const struct mailbox_update *update,
+				    struct mail_index_transaction *trans)
 {
 	struct mdbox_mailbox *mbox = (struct mdbox_mailbox *)box;
-	struct mail_index_transaction *trans;
+	struct mail_index_transaction *new_trans = NULL;
 	const struct mail_index_header *hdr;
 	uint32_t uid_validity, uid_next;
 
 	if (mdbox_map_open_or_create(mbox->storage->map) < 0)
 		return -1;
 
+	if (trans == NULL) {
+		new_trans = mail_index_transaction_begin(box->view, 0);
+		trans = new_trans;
+	}
+
 	hdr = mail_index_get_header(box->view);
-	trans = mail_index_transaction_begin(box->view, 0);
-
 	uid_validity = hdr->uid_validity;
 	if (update != NULL && update->uid_validity != 0)
 		uid_validity = update->uid_validity;
@@ -226,21 +230,24 @@
 	}
 
 	mdbox_update_header(mbox, trans, update);
-	if (mail_index_transaction_commit(&trans) < 0) {
-		mail_storage_set_index_error(box);
-		return -1;
+	if (new_trans != NULL) {
+		if (mail_index_transaction_commit(&new_trans) < 0) {
+			mail_storage_set_index_error(box);
+			return -1;
+		}
 	}
 	return 0;
 }
 
 static int mdbox_mailbox_create_indexes(struct mailbox *box,
-					const struct mailbox_update *update)
+					const struct mailbox_update *update,
+					struct mail_index_transaction *trans)
 {
 	struct mdbox_mailbox *mbox = (struct mdbox_mailbox *)box;
 	int ret;
 
 	mbox->creating = TRUE;
-	ret = mdbox_write_index_header(box, update);
+	ret = mdbox_write_index_header(box, update, trans);
 	mbox->creating = FALSE;
 	return ret;
 }
@@ -280,7 +287,7 @@
 
 	if (mail_guid_128_is_empty(hdr.mailbox_guid)) {
 		/* regenerate it */
-		if (mdbox_write_index_header(box, NULL) < 0 ||
+		if (mdbox_write_index_header(box, NULL, NULL) < 0 ||
 		    mdbox_read_header(mbox, &hdr) < 0)
 			return -1;
 	}
@@ -297,7 +304,7 @@
 	}
 	if (update->cache_fields != NULL)
 		index_storage_mailbox_update_cache_fields(box, update);
-	return mdbox_write_index_header(box, update);
+	return mdbox_write_index_header(box, update, NULL);
 }
 
 static int mdbox_mailbox_unref_mails(struct mdbox_mailbox *mbox)
diff -r 6b35189988dc -r 7fc5db26f6eb src/lib-storage/index/dbox-multi/mdbox-sync.c
--- a/src/lib-storage/index/dbox-multi/mdbox-sync.c	Fri Jun 04 19:57:49 2010 +0100
+++ b/src/lib-storage/index/dbox-multi/mdbox-sync.c	Fri Jun 04 21:09:12 2010 +0100
@@ -207,7 +207,9 @@
 	int ret;
 	bool rebuild, storage_rebuilt = FALSE;
 
-	rebuild = mdbox_refresh_header(mbox, TRUE) < 0 ||
+	/* avoid race conditions with mailbox creation, don't check for dbox
+	   headers until syncing has locked the mailbox */
+	rebuild = mbox->storage->corrupted ||
 		(flags & MDBOX_SYNC_FLAG_FORCE_REBUILD) != 0;
 	if (rebuild) {
 		if (mdbox_storage_rebuild(mbox->storage) < 0)
diff -r 6b35189988dc -r 7fc5db26f6eb src/lib-storage/index/dbox-single/sdbox-storage.c
--- a/src/lib-storage/index/dbox-single/sdbox-storage.c	Fri Jun 04 19:57:49 2010 +0100
+++ b/src/lib-storage/index/dbox-single/sdbox-storage.c	Fri Jun 04 21:09:12 2010 +0100
@@ -128,16 +128,20 @@
 }
 
 static int sdbox_write_index_header(struct mailbox *box,
-				    const struct mailbox_update *update)
+				    const struct mailbox_update *update,
+				    struct mail_index_transaction *trans)
 {
 	struct sdbox_mailbox *mbox = (struct sdbox_mailbox *)box;


More information about the dovecot-cvs mailing list