From: Timo Sirainen Date: Thu, 28 Aug 2014 17:14:43 +0000 (+0900) Subject: lib-index, lib-storage: Fixed race conditions with deleting mailbox. X-Git-Tag: 2.2.14.rc1~112 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f90cbe597c41d5cc91debd371f8648bd8e6ffbc2;p=thirdparty%2Fdovecot%2Fcore.git lib-index, lib-storage: Fixed race conditions with deleting mailbox. Now only one process can successfully finish mailbox_mark_index_deleted(). --- diff --git a/src/lib-index/mail-index-sync.c b/src/lib-index/mail-index-sync.c index 42a9b1388d..afea7a3e28 100644 --- a/src/lib-index/mail-index-sync.c +++ b/src/lib-index/mail-index-sync.c @@ -360,21 +360,13 @@ mail_index_sync_begin_init(struct mail_index *index, } } - if (!mail_index_need_sync(index, flags, - log_file_seq, log_file_offset)) { + if (!mail_index_need_sync(index, flags, log_file_seq, log_file_offset) && + !index->index_deleted) { if (locked) mail_transaction_log_sync_unlock(index->log); return 0; } - if (index->index_deleted && - (flags & MAIL_INDEX_SYNC_FLAG_DELETING_INDEX) == 0) { - /* index is already deleted. we can't sync. */ - if (locked) - mail_transaction_log_sync_unlock(index->log); - return -1; - } - if (!locked) { /* it looks like we have something to sync. lock the file and check again. */ @@ -383,6 +375,14 @@ mail_index_sync_begin_init(struct mail_index *index, log_file_offset); } + if (index->index_deleted && + (flags & MAIL_INDEX_SYNC_FLAG_DELETING_INDEX) == 0) { + /* index is already deleted. we can't sync. */ + if (locked) + mail_transaction_log_sync_unlock(index->log); + return -1; + } + hdr = &index->map->hdr; if (hdr->log_file_tail_offset > hdr->log_file_head_offset || hdr->log_file_seq > seq || @@ -482,7 +482,8 @@ mail_index_sync_begin_to2(struct mail_index *index, ctx->ext_trans = mail_index_transaction_begin(ctx->view, trans_flags); ctx->ext_trans->sync_transaction = TRUE; ctx->ext_trans->commit_deleted_index = - (flags & MAIL_INDEX_SYNC_FLAG_DELETING_INDEX) != 0; + (flags & (MAIL_INDEX_SYNC_FLAG_DELETING_INDEX | + MAIL_INDEX_SYNC_FLAG_TRY_DELETING_INDEX)) != 0; *ctx_r = ctx; *view_r = ctx->view; @@ -789,10 +790,16 @@ int mail_index_sync_commit(struct mail_index_sync_ctx **_ctx) index_undeleted = ctx->ext_trans->index_undeleted; delete_index = index->index_delete_requested && !index_undeleted && - (ctx->flags & MAIL_INDEX_SYNC_FLAG_DELETING_INDEX) != 0; + (ctx->flags & (MAIL_INDEX_SYNC_FLAG_DELETING_INDEX | + MAIL_INDEX_SYNC_FLAG_TRY_DELETING_INDEX)) != 0; if (delete_index) { /* finish this sync by marking the index deleted */ mail_index_set_deleted(ctx->ext_trans); + } else if (index->index_deleted && !index_undeleted && + (ctx->flags & MAIL_INDEX_SYNC_FLAG_TRY_DELETING_INDEX) == 0) { + /* another process just marked the index deleted. + finish the sync, but return error. */ + ret = -1; } mail_index_sync_update_mailbox_offset(ctx); diff --git a/src/lib-index/mail-index.h b/src/lib-index/mail-index.h index e3a336277d..e4ddf8dd6f 100644 --- a/src/lib-index/mail-index.h +++ b/src/lib-index/mail-index.h @@ -159,7 +159,11 @@ enum mail_index_sync_flags { MAIL_INDEX_SYNC_FLAG_FSYNC = 0x10, /* If we see "delete index" request transaction, finish it. This flag also allows committing more changes to a deleted index. */ - MAIL_INDEX_SYNC_FLAG_DELETING_INDEX = 0x20 + MAIL_INDEX_SYNC_FLAG_DELETING_INDEX = 0x20, + /* Same as MAIL_INDEX_SYNC_FLAG_DELETING_INDEX, but finish index + deletion only once and fail the rest (= avoid race conditions when + multiple processes try to mark the index deleted) */ + MAIL_INDEX_SYNC_FLAG_TRY_DELETING_INDEX = 0x40 }; enum mail_index_view_sync_flags { diff --git a/src/lib-index/mail-transaction-log-file.c b/src/lib-index/mail-transaction-log-file.c index 161d0665a6..f42b4e224f 100644 --- a/src/lib-index/mail-transaction-log-file.c +++ b/src/lib-index/mail-transaction-log-file.c @@ -1284,6 +1284,7 @@ log_file_track_sync(struct mail_transaction_log_file *file, if (file->sync_offset < file->index_undeleted_offset) break; file->log->index->index_deleted = TRUE; + file->log->index->index_delete_requested = FALSE; file->index_deleted_offset = file->sync_offset + trans_size; break; case MAIL_TRANSACTION_INDEX_UNDELETED: diff --git a/src/lib-storage/index/index-sync.c b/src/lib-storage/index/index-sync.c index 49a44b0b8d..53c1fd2779 100644 --- a/src/lib-storage/index/index-sync.c +++ b/src/lib-storage/index/index-sync.c @@ -17,8 +17,11 @@ enum mail_index_sync_flags index_storage_get_sync_flags(struct mailbox *box) if ((box->flags & MAILBOX_FLAG_DROP_RECENT) != 0) sync_flags |= MAIL_INDEX_SYNC_FLAG_DROP_RECENT; - if (box->deleting) - sync_flags |= MAIL_INDEX_SYNC_FLAG_DELETING_INDEX; + if (box->deleting) { + sync_flags |= box->delete_sync_check ? + MAIL_INDEX_SYNC_FLAG_TRY_DELETING_INDEX : + MAIL_INDEX_SYNC_FLAG_DELETING_INDEX; + } return sync_flags; } diff --git a/src/lib-storage/mail-storage-private.h b/src/lib-storage/mail-storage-private.h index e30ef466fe..a10e6c6072 100644 --- a/src/lib-storage/mail-storage-private.h +++ b/src/lib-storage/mail-storage-private.h @@ -332,6 +332,8 @@ struct mailbox { unsigned int creating:1; /* Mailbox is being deleted */ unsigned int deleting:1; + /* Don't use MAIL_INDEX_SYNC_FLAG_DELETING_INDEX for sync flag */ + unsigned int delete_sync_check:1; /* Delete mailbox only if it's empty */ unsigned int deleting_must_be_empty:1; /* The backend wants to skip checking if there are 0 messages before diff --git a/src/lib-storage/mail-storage.c b/src/lib-storage/mail-storage.c index e8ea62a1d1..1f2c4ccc7a 100644 --- a/src/lib-storage/mail-storage.c +++ b/src/lib-storage/mail-storage.c @@ -1319,7 +1319,10 @@ int mailbox_mark_index_deleted(struct mailbox *box, bool del) /* sync the mailbox. this finishes the index deletion and it can succeed only for a single session. we do it here, so the rest of the deletion code doesn't have to worry about race conditions. */ - if (mailbox_sync(box, MAILBOX_SYNC_FLAG_FULL_READ) < 0) + box->delete_sync_check = TRUE; + ret = mailbox_sync(box, MAILBOX_SYNC_FLAG_FULL_READ); + box->delete_sync_check = FALSE; + if (ret < 0) return -1; box->marked_deleted = del;