dovecot-2.2: mailbox_list_index: Removed race conditions from in...
dovecot at dovecot.org
dovecot at dovecot.org
Wed Feb 20 10:58:15 EET 2013
details: http://hg.dovecot.org/dovecot-2.2/rev/5a068b9c04a9
changeset: 15853:5a068b9c04a9
user: Timo Sirainen <tss at iki.fi>
date: Wed Feb 20 10:58:02 2013 +0200
description:
mailbox_list_index: Removed race conditions from index updates.
diffstat:
src/lib-storage/list/mailbox-list-index-status.c | 350 +++++++++++++---------
1 files changed, 203 insertions(+), 147 deletions(-)
diffs (truncated from 448 to 300 lines):
diff -r 61a61657d887 -r 5a068b9c04a9 src/lib-storage/list/mailbox-list-index-status.c
--- a/src/lib-storage/list/mailbox-list-index-status.c Wed Feb 20 10:57:13 2013 +0200
+++ b/src/lib-storage/list/mailbox-list-index-status.c Wed Feb 20 10:58:02 2013 +0200
@@ -10,6 +10,16 @@
(STATUS_MESSAGES | STATUS_UNSEEN | STATUS_RECENT | \
STATUS_UIDNEXT | STATUS_UIDVALIDITY | STATUS_HIGHESTMODSEQ)
+struct index_list_changes {
+ struct mailbox_status status;
+ guid_128_t guid;
+ uint32_t seq;
+
+ bool rec_changed;
+ bool msgs_changed;
+ bool hmodseq_changed;
+};
+
struct index_list_storage_module index_list_storage_module =
MODULE_CONTEXT_INIT(&mail_storage_module_register);
@@ -192,210 +202,259 @@
return ibox->module_ctx.super.get_metadata(box, items, metadata_r);
}
-static int
-index_list_update_full(struct mailbox *box, struct mail_index_view *view,
- uint32_t seq, const guid_128_t new_guid,
- const struct mailbox_status *new_status,
- bool update_backend)
+static bool
+index_list_update_fill_changes(struct mailbox *box,
+ struct mail_index_view *list_view,
+ struct index_list_changes *changes_r)
+{
+ struct mailbox_list_index_node *node;
+ struct mail_index_view *view;
+ const struct mail_index_header *hdr;
+ struct mailbox_metadata metadata;
+ uint32_t seq1, seq2;
+
+ memset(changes_r, 0, sizeof(*changes_r));
+
+ node = mailbox_list_index_lookup(box->list, box->name);
+ if (node == NULL)
+ return FALSE;
+ if (!mail_index_lookup_seq(list_view, node->uid, &changes_r->seq))
+ return FALSE;
+
+ /* get STATUS info using the latest data in index.
+ note that for shared mailboxes (with private indexes) this
+ also means that the unseen count is always the owner's
+ count, not what exists in the private index. */
+ view = mail_index_view_open(box->index);
+ hdr = mail_index_get_header(view);
+
+ changes_r->status.messages = hdr->messages_count;
+ changes_r->status.unseen =
+ hdr->messages_count - hdr->seen_messages_count;
+ changes_r->status.uidvalidity = hdr->uid_validity;
+ changes_r->status.uidnext = hdr->next_uid;
+
+ if (!mail_index_lookup_seq_range(view, hdr->first_recent_uid,
+ (uint32_t)-1, &seq1, &seq2))
+ changes_r->status.recent = 0;
+ else
+ changes_r->status.recent = seq2 - seq1 + 1;
+
+ changes_r->status.highest_modseq = mail_index_modseq_get_highest(view);
+ if (changes_r->status.highest_modseq == 0) {
+ /* modseqs not enabled yet, but we can't return 0 */
+ changes_r->status.highest_modseq = 1;
+ }
+ mail_index_view_close(&view); hdr = NULL;
+
+ if (mailbox_get_metadata(box, MAILBOX_METADATA_GUID, &metadata) == 0)
+ memcpy(changes_r->guid, metadata.guid, sizeof(changes_r->guid));
+ return TRUE;
+}
+
+static bool
+index_list_has_changed(struct mailbox *box, struct mail_index_view *list_view,
+ struct index_list_changes *changes)
{
struct mailbox_list_index *ilist = INDEX_LIST_CONTEXT(box->list);
- struct mail_index_transaction *trans;
- struct mail_index_transaction_commit_result result;
struct mailbox_status old_status;
guid_128_t old_guid;
- bool rec_changed, msgs_changed, hmodseq_changed;
memset(&old_status, 0, sizeof(old_status));
memset(old_guid, 0, sizeof(old_guid));
- (void)mailbox_list_index_status(box->list, view, seq, CACHED_STATUS_ITEMS,
+ (void)mailbox_list_index_status(box->list, list_view, changes->seq,
+ CACHED_STATUS_ITEMS,
&old_status, old_guid);
- rec_changed = old_status.uidvalidity != new_status->uidvalidity &&
- new_status->uidvalidity != 0;
- if (!guid_128_equals(new_guid, old_guid) &&
- !guid_128_is_empty(new_guid))
- rec_changed = TRUE;
- msgs_changed = old_status.messages != new_status->messages ||
- old_status.unseen != new_status->unseen ||
- old_status.recent != new_status->recent ||
- old_status.uidnext != new_status->uidnext;
+ changes->rec_changed =
+ old_status.uidvalidity != changes->status.uidvalidity &&
+ changes->status.uidvalidity != 0;
+ if (!guid_128_equals(changes->guid, old_guid) &&
+ !guid_128_is_empty(changes->guid))
+ changes->rec_changed = TRUE;
+ changes->msgs_changed =
+ old_status.messages != changes->status.messages ||
+ old_status.unseen != changes->status.unseen ||
+ old_status.recent != changes->status.recent ||
+ old_status.uidnext != changes->status.uidnext;
/* update highest-modseq only if they're ever been used */
- if (old_status.highest_modseq == new_status->highest_modseq) {
- hmodseq_changed = FALSE;
+ if (old_status.highest_modseq == changes->status.highest_modseq) {
+ changes->hmodseq_changed = FALSE;
} else if ((box->enabled_features & MAILBOX_FEATURE_CONDSTORE) != 0 ||
old_status.highest_modseq != 0) {
- hmodseq_changed = TRUE;
+ changes->hmodseq_changed = TRUE;
} else {
const void *data;
bool expunged;
- mail_index_lookup_ext(view, seq, ilist->hmodseq_ext_id,
- &data, &expunged);
- hmodseq_changed = data != NULL;
+ mail_index_lookup_ext(list_view, changes->seq,
+ ilist->hmodseq_ext_id, &data, &expunged);
+ changes->hmodseq_changed = data != NULL;
}
- if (hmodseq_changed &&
- old_status.highest_modseq != new_status->highest_modseq)
- hmodseq_changed = TRUE;
+ if (changes->hmodseq_changed &&
+ old_status.highest_modseq != changes->status.highest_modseq)
+ changes->hmodseq_changed = TRUE;
- if (!rec_changed && !msgs_changed && !hmodseq_changed)
- return 0;
+ return changes->rec_changed || changes->msgs_changed ||
+ changes->hmodseq_changed;
+}
- trans = mail_index_transaction_begin(view,
- MAIL_INDEX_TRANSACTION_FLAG_EXTERNAL);
+static void
+index_list_update(struct mailbox *box, struct mail_index_view *list_view,
+ struct mail_index_transaction *list_trans,
+ const struct index_list_changes *changes)
+{
+ struct mailbox_list_index *ilist = INDEX_LIST_CONTEXT(box->list);
- if (rec_changed) {
+ if (changes->rec_changed) {
struct mailbox_list_index_record rec;
const void *old_data;
bool expunged;
- mail_index_lookup_ext(view, seq, ilist->ext_id,
+ mail_index_lookup_ext(list_view, changes->seq, ilist->ext_id,
&old_data, &expunged);
i_assert(old_data != NULL);
memcpy(&rec, old_data, sizeof(rec));
- if (new_status->uidvalidity != 0)
- rec.uid_validity = new_status->uidvalidity;
- if (!guid_128_is_empty(new_guid))
- memcpy(rec.guid, new_guid, sizeof(rec.guid));
- mail_index_update_ext(trans, seq, ilist->ext_id, &rec, NULL);
+ if (changes->status.uidvalidity != 0)
+ rec.uid_validity = changes->status.uidvalidity;
+ if (!guid_128_is_empty(changes->guid))
+ memcpy(rec.guid, changes->guid, sizeof(rec.guid));
+ mail_index_update_ext(list_trans, changes->seq, ilist->ext_id,
+ &rec, NULL);
}
- if (msgs_changed) {
+ if (changes->msgs_changed) {
struct mailbox_list_index_msgs_record msgs;
memset(&msgs, 0, sizeof(msgs));
- msgs.messages = new_status->messages;
- msgs.unseen = new_status->unseen;
- msgs.recent = new_status->recent;
- msgs.uidnext = new_status->uidnext;
+ msgs.messages = changes->status.messages;
+ msgs.unseen = changes->status.unseen;
+ msgs.recent = changes->status.recent;
+ msgs.uidnext = changes->status.uidnext;
- mail_index_update_ext(trans, seq, ilist->msgs_ext_id,
- &msgs, NULL);
+ mail_index_update_ext(list_trans, changes->seq,
+ ilist->msgs_ext_id, &msgs, NULL);
}
- if (hmodseq_changed) {
- mail_index_update_ext(trans, seq, ilist->hmodseq_ext_id,
- &new_status->highest_modseq, NULL);
+ if (changes->hmodseq_changed) {
+ mail_index_update_ext(list_trans, changes->seq,
+ ilist->hmodseq_ext_id,
+ &changes->status.highest_modseq, NULL);
+ }
+}
+
+static int index_list_update_mailbox(struct mailbox *box)
+{
+ struct mailbox_list_index *ilist = INDEX_LIST_CONTEXT(box->list);
+ struct mail_index_sync_ctx *list_sync_ctx;
+ struct mail_index_view *list_view;
+ struct mail_index_transaction *list_trans;
+ struct index_list_changes changes;
+ int ret;
+
+ i_assert(box->opened);
+
+ if (ilist->syncing || ilist->updating_status)
+ return 0;
+
+ /* refresh the mailbox list index once. we can't do this again after
+ locking, because it could trigger list syncing. */
+ (void)mailbox_list_index_refresh(box->list);
+
+ /* first do a quick check while unlocked to see if anything changes */
+ list_view = mail_index_view_open(ilist->index);
+ if (!index_list_update_fill_changes(box, list_view, &changes))
+ ret = -1;
+ else if (!index_list_has_changed(box, list_view, &changes))
+ ret = 0;
+ else
+ ret = 1;
+ mail_index_view_close(&list_view);
+ if (ret <= 0) {
+ if (ret < 0)
+ mailbox_list_index_refresh_later(box->list);
+ return 0;
}
- if (update_backend) {
- if (box->v.list_index_update_sync != NULL)
- box->v.list_index_update_sync(box, trans, seq);
+ /* looks like there are some changes. now lock the list index and do
+ the whole thing all over again while locked. this guarantees
+ that we'll always write the latest state of the mailbox. */
+ if (mail_index_sync_begin(ilist->index, &list_sync_ctx,
+ &list_view, &list_trans, 0) < 0) {
+ mailbox_set_index_error(box);
+ return -1;
+ }
+ /* refresh to latest state of the mailbox now that we're locked */
+ if (mail_index_refresh(box->index) < 0) {
+ mailbox_set_index_error(box);
+ mail_index_sync_rollback(&list_sync_ctx);
+ return -1;
}
- if (mail_index_transaction_commit_full(&trans, &result) < 0) {
- mailbox_list_index_set_index_error(box->list);
+ if (!index_list_update_fill_changes(box, list_view, &changes))
+ mailbox_list_index_refresh_later(box->list);
+ else if (index_list_has_changed(box, list_view, &changes)) {
+ ilist->updating_status = TRUE;
+ index_list_update(box, list_view, list_trans, &changes);
+ if (box->v.list_index_update_sync != NULL) {
+ box->v.list_index_update_sync(box, list_trans,
+ changes.seq);
+ }
+ ilist->updating_status = FALSE;
+ }
+
+ if (mail_index_sync_commit(&list_sync_ctx) < 0) {
+ mailbox_set_index_error(box);
return -1;
}
return 0;
}
-static int
-index_list_update(struct mailbox *box, struct mail_index_view *view,
- uint32_t seq, const struct mailbox_status *status)
-{
- struct mailbox_metadata metadata;
-
- i_assert(box->opened);
-
- if (mailbox_get_metadata(box, MAILBOX_METADATA_GUID, &metadata) < 0)
- memset(&metadata, 0, sizeof(metadata));
- return index_list_update_full(box, view, seq, metadata.guid,
- status, TRUE);
-}
-
More information about the dovecot-cvs
mailing list