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