dbox, mdbox: Fixed race conditions when creating mailboxes.

--HG--
branch : HEAD

diff --git a/src/lib-storage/index/dbox-common/dbox-storage.c b/src/lib-storage/index/dbox-common/dbox-storage.c
index 8905bf984cc0bebd2457dac201491eb3302b32f0..5fb7b0a132eb3086d6846b1634c7b5dc10775c6d 100644 (file)
--- a/src/lib-storage/index/dbox-common/dbox-storage.c
+++ b/src/lib-storage/index/dbox-common/dbox-storage.c
@@ -94,6 +94,10 @@ int dbox_mailbox_create(struct mailbox *box,
                        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 @@ int dbox_mailbox_create(struct mailbox *box,
 
        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 --git a/src/lib-storage/index/dbox-common/dbox-storage.h b/src/lib-storage/index/dbox-common/dbox-storage.h
index 5f22639552eab741735bf216b59369dfc12ee169..782932203b19ec466049ff7b40fc8b77b48475d8 100644 (file)
--- a/src/lib-storage/index/dbox-common/dbox-storage.h
+++ b/src/lib-storage/index/dbox-common/dbox-storage.h
@@ -37,7 +37,8 @@ struct dbox_storage_vfuncs {
                         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 --git a/src/lib-storage/index/dbox-multi/mdbox-map-private.h b/src/lib-storage/index/dbox-multi/mdbox-map-private.h
index 59a6a8151d8502c06dd34700c765ef45b8e216a2..7d1128dd013727abf641e0e50726ba8447e53332 100644 (file)
--- a/src/lib-storage/index/dbox-multi/mdbox-map-private.h
+++ b/src/lib-storage/index/dbox-multi/mdbox-map-private.h
@@ -16,7 +16,6 @@ struct mdbox_map {
 
        struct mail_index *index;
        struct mail_index_view *view;
-       uint32_t created_uid_validity;
 
        uint32_t map_ext_id, ref_ext_id;
 

diff --git a/src/lib-storage/index/dbox-multi/mdbox-map.c b/src/lib-storage/index/dbox-multi/mdbox-map.c
index 696f0e9c8f483cf95ef864c8597f04d55f160353..3953c80d5eeef231d69b63830ca1fb50558ad759 100644 (file)
--- a/src/lib-storage/index/dbox-multi/mdbox-map.c
+++ b/src/lib-storage/index/dbox-multi/mdbox-map.c
@@ -30,6 +30,8 @@ struct mdbox_map_transaction_context {
        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 @@ mdbox_map_init(struct mdbox_storage *storage, struct mailbox_list *root_list,
                                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 @@ static int mdbox_map_open_internal(struct mdbox_map *map, bool create_missing)
 
        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 @@ void mdbox_map_append_free(struct mdbox_map_append_context **_ctx)
        i_free(ctx);
 }
 
-uint32_t mdbox_map_get_uid_validity(struct mdbox_map *map)
+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;
 
-       i_assert(map->view != NULL);
+       /* 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)
-               return hdr->uid_validity;
+       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);
+}
 
-       /* 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;
+uint32_t mdbox_map_get_uid_validity(struct mdbox_map *map)
+{
+       uint32_t uid_validity;
+
+       i_assert(map->view != NULL);
+
+       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 --git a/src/lib-storage/index/dbox-multi/mdbox-map.h b/src/lib-storage/index/dbox-multi/mdbox-map.h
index b2892a8766aa50ebc71628205781d12c3b50d1d5..1033c60e8bdfcee4dde2de193bb83eef283fba10 100644 (file)
--- a/src/lib-storage/index/dbox-multi/mdbox-map.h
+++ b/src/lib-storage/index/dbox-multi/mdbox-map.h
@@ -101,8 +101,7 @@ int mdbox_map_append_move(struct mdbox_map_append_context *ctx,
 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 --git a/src/lib-storage/index/dbox-multi/mdbox-storage-rebuild.c b/src/lib-storage/index/dbox-multi/mdbox-storage-rebuild.c
index 9a3f337ac60eba136356b015d7fba5c0c4f99a52..dd4137d6952c2ac73ae96a3573bb633d857dd834 100644 (file)
--- a/src/lib-storage/index/dbox-multi/mdbox-storage-rebuild.c
+++ b/src/lib-storage/index/dbox-multi/mdbox-storage-rebuild.c
@@ -812,8 +812,6 @@ mdbox_storage_rebuild_scan_dir(struct mdbox_storage_rebuild_context *ctx,
 
 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 @@ static int mdbox_storage_rebuild_scan(struct mdbox_storage_rebuild_context *ctx)
 
        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 --git a/src/lib-storage/index/dbox-multi/mdbox-storage.c b/src/lib-storage/index/dbox-multi/mdbox-storage.c
index 11f695b877792a83ff82d586dea57efca38613f6..12148f3697fc626b3aad4d48ecfc0ca084a7d4df 100644 (file)
--- a/src/lib-storage/index/dbox-multi/mdbox-storage.c
+++ b/src/lib-storage/index/dbox-multi/mdbox-storage.c
@@ -182,19 +182,23 @@ void mdbox_update_header(struct mdbox_mailbox *mbox,
 }
 
 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;
 
-       hdr = mail_index_get_header(box->view);
-       trans = mail_index_transaction_begin(box->view, 0);
+       if (trans == NULL) {
+               new_trans = mail_index_transaction_begin(box->view, 0);
+               trans = new_trans;
+       }
 
+       hdr = mail_index_get_header(box->view);
        uid_validity = hdr->uid_validity;
        if (update != NULL && update->uid_validity != 0)
                uid_validity = update->uid_validity;
@@ -226,21 +230,24 @@ static int mdbox_write_index_header(struct mailbox *box,
        }
 
        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 @@ mdbox_mailbox_get_guid(struct mailbox *box, uint8_t guid[MAIL_GUID_128_SIZE])
 
        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 @@ mdbox_mailbox_update(struct mailbox *box, const struct mailbox_update *update)
        }
        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 --git a/src/lib-storage/index/dbox-multi/mdbox-sync.c b/src/lib-storage/index/dbox-multi/mdbox-sync.c
index c23d0ffa62e8f68fbf3e006ab1375523db768aa3..51046f93c61778685643033729bf051f4c490f7b 100644 (file)
--- a/src/lib-storage/index/dbox-multi/mdbox-sync.c
+++ b/src/lib-storage/index/dbox-multi/mdbox-sync.c
@@ -207,7 +207,9 @@ int mdbox_sync_begin(struct mdbox_mailbox *mbox, enum mdbox_sync_flags flags,
        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 --git a/src/lib-storage/index/dbox-single/sdbox-storage.c b/src/lib-storage/index/dbox-single/sdbox-storage.c
index 1db00724a9b62d8556049662493c23bea62f6cad..4150c60bd8b11bae93316bf8176fde95bd07e332 100644 (file)
--- a/src/lib-storage/index/dbox-single/sdbox-storage.c
+++ b/src/lib-storage/index/dbox-single/sdbox-storage.c
@@ -128,16 +128,20 @@ void sdbox_update_header(struct sdbox_mailbox *mbox,
 }
 
 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;
-       struct mail_index_transaction *trans;
+       struct mail_index_transaction *new_trans = NULL;
        const struct mail_index_header *hdr;
        uint32_t uid_validity, uid_next;
 
-       hdr = mail_index_get_header(box->view);
-       trans = mail_index_transaction_begin(box->view, 0);
+       if (trans == NULL) {
+               new_trans = mail_index_transaction_begin(box->view, 0);
+               trans = new_trans;
+       }
 
+       hdr = mail_index_get_header(box->view);
        if (update != NULL && update->uid_validity != 0)
                uid_validity = update->uid_validity;
        else if (hdr->uid_validity != 0)
@@ -170,22 +174,25 @@ static int sdbox_write_index_header(struct mailbox *box,
        }
 
        sdbox_update_header(mbox, trans, update);
-       if (mail_index_transaction_commit(&trans) < 0) {
-               mail_storage_set_internal_error(box->storage);
-               mail_index_reset_error(box->index);
-               return -1;
+       if (new_trans != NULL) {
+               if (mail_index_transaction_commit(&new_trans) < 0) {
+                       mail_storage_set_internal_error(box->storage);
+                       mail_index_reset_error(box->index);
+                       return -1;
+               }
        }
        return 0;
 }
 
 static int sdbox_mailbox_create_indexes(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;
        int ret;
 
        mbox->creating = TRUE;
-       ret = sdbox_write_index_header(box, update);
+       ret = sdbox_write_index_header(box, update, trans);
        mbox->creating = FALSE;
        return ret;
 }
@@ -206,7 +213,7 @@ sdbox_mailbox_get_guid(struct mailbox *box, uint8_t guid[MAIL_GUID_128_SIZE])
 
        if (mail_guid_128_is_empty(hdr.mailbox_guid)) {
                /* regenerate it */
-               if (sdbox_write_index_header(box, NULL) < 0 ||
+               if (sdbox_write_index_header(box, NULL, NULL) < 0 ||
                    sdbox_read_header(mbox, &hdr, TRUE) < 0)
                        return -1;
        }
@@ -223,7 +230,7 @@ dbox_mailbox_update(struct mailbox *box, const struct mailbox_update *update)
        }
        if (update->cache_fields != NULL)
                index_storage_mailbox_update_cache_fields(box, update);
-       return sdbox_write_index_header(box, update);
+       return sdbox_write_index_header(box, update, NULL);
 }
 
 struct mail_storage dbox_storage = {