From: Timo Sirainen Date: Fri, 4 Jun 2010 20:09:12 +0000 (+0100) Subject: dbox, mdbox: Fixed race conditions when creating mailboxes. X-Git-Tag: 2.0.beta6~46 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=dd4f30895ebbddd77e000472fbadcb3128ae2883;p=thirdparty%2Fdovecot%2Fcore.git 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 8905bf984c..5fb7b0a132 100644 --- 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 5f22639552..782932203b 100644 --- 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 59a6a8151d..7d1128dd01 100644 --- 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 696f0e9c8f..3953c80d5e 100644 --- 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 b2892a8766..1033c60e8b 100644 --- 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 9a3f337ac6..dd4137d695 100644 --- 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 11f695b877..12148f3697 100644 --- 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 c23d0ffa62..51046f93c6 100644 --- 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 1db00724a9..4150c60bd8 100644 --- 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 = {