# HG changeset patch # User Timo Sirainen # Date 1241469839 14400 # Node ID 0bb192fe0abdfe749d45db77de79dfa1cd4575e2 # Parent 0aa17f3e4a6dbe26e95cc8682b5818154c870939 Maildir: More fixes to uidlist handling. diff -r 0aa17f3e4a6d -r 0bb192fe0abd src/lib-storage/index/maildir/maildir-save.c --- a/src/lib-storage/index/maildir/maildir-save.c Mon May 04 16:42:43 2009 -0400 +++ b/src/lib-storage/index/maildir/maildir-save.c Mon May 04 16:43:59 2009 -0400 @@ -691,7 +691,7 @@ return -1; } - ctx->locked = ret > 0; + ctx->locked = maildir_uidlist_is_locked(ctx->mbox->uidlist); if (ctx->locked) { if (maildir_transaction_save_commit_pre_sync(ctx) < 0) { maildir_transaction_save_rollback(ctx); @@ -761,7 +761,8 @@ if (ctx->uidlist_sync_ctx != NULL) { /* update dovecot-uidlist file. */ - if (maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx) < 0) + if (maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx, + ret >= 0) < 0) ret = -1; } @@ -844,7 +845,7 @@ maildir_transaction_unlink_copied_files(ctx, NULL); if (ctx->uidlist_sync_ctx != NULL) - (void)maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx); + (void)maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx, FALSE); if (ctx->locked) maildir_uidlist_unlock(ctx->mbox->uidlist); if (ctx->sync_ctx != NULL) diff -r 0aa17f3e4a6d -r 0bb192fe0abd src/lib-storage/index/maildir/maildir-sync-index.c --- a/src/lib-storage/index/maildir/maildir-sync-index.c Mon May 04 16:42:43 2009 -0400 +++ b/src/lib-storage/index/maildir/maildir-sync-index.c Mon May 04 16:43:59 2009 -0400 @@ -556,7 +556,8 @@ mail_index_view_close(&view2); if (ctx->uidlist_sync_ctx != NULL) { - if (maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx) < 0) + if (maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx, + TRUE) < 0) ret = -1; } diff -r 0aa17f3e4a6d -r 0bb192fe0abd src/lib-storage/index/maildir/maildir-sync.c --- a/src/lib-storage/index/maildir/maildir-sync.c Mon May 04 16:42:43 2009 -0400 +++ b/src/lib-storage/index/maildir/maildir-sync.c Mon May 04 16:43:59 2009 -0400 @@ -266,7 +266,7 @@ static void maildir_sync_deinit(struct maildir_sync_context *ctx) { if (ctx->uidlist_sync_ctx != NULL) - (void)maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx); + (void)maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx, FALSE); if (ctx->index_sync_ctx != NULL) { (void)maildir_sync_index_finish(&ctx->index_sync_ctx, TRUE, FALSE); @@ -866,7 +866,7 @@ } } - return maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx); + return maildir_uidlist_sync_deinit(&ctx->uidlist_sync_ctx, TRUE); } int maildir_storage_sync_force(struct maildir_mailbox *mbox, uint32_t uid) diff -r 0aa17f3e4a6d -r 0bb192fe0abd src/lib-storage/index/maildir/maildir-uidlist.c --- a/src/lib-storage/index/maildir/maildir-uidlist.c Mon May 04 16:42:43 2009 -0400 +++ b/src/lib-storage/index/maildir/maildir-uidlist.c Mon May 04 16:43:59 2009 -0400 @@ -89,9 +89,9 @@ unsigned int recreate:1; unsigned int initial_read:1; unsigned int initial_hdr_read:1; - unsigned int initial_sync:1; unsigned int retry_rewind:1; unsigned int locked_refresh:1; + unsigned int unsorted:1; }; struct maildir_uidlist_sync_ctx { @@ -125,7 +125,8 @@ struct maildir_uidlist_rec **rec_r); static int maildir_uidlist_lock_timeout(struct maildir_uidlist *uidlist, - bool nonblock, bool refresh) + bool nonblock, bool refresh, + bool refresh_when_locked) { struct mailbox *box = &uidlist->ibox->box; const char *control_dir, *path; @@ -135,6 +136,10 @@ int i, ret; if (uidlist->lock_count > 0) { + if (!uidlist->locked_refresh && refresh_when_locked) { + if (maildir_uidlist_refresh(uidlist) < 0) + return -1; + } uidlist->lock_count++; return 1; } @@ -187,12 +192,12 @@ int maildir_uidlist_lock(struct maildir_uidlist *uidlist) { - return maildir_uidlist_lock_timeout(uidlist, FALSE, TRUE); + return maildir_uidlist_lock_timeout(uidlist, FALSE, TRUE, FALSE); } int maildir_uidlist_try_lock(struct maildir_uidlist *uidlist) { - return maildir_uidlist_lock_timeout(uidlist, TRUE, TRUE); + return maildir_uidlist_lock_timeout(uidlist, TRUE, TRUE, FALSE); } int maildir_uidlist_lock_touch(struct maildir_uidlist *uidlist) @@ -292,6 +297,16 @@ uidlist->read_line_count = 0; } +static void maildir_uidlist_reset(struct maildir_uidlist *uidlist) +{ + maildir_uidlist_close(uidlist); + uidlist->last_seen_uid = 0; + uidlist->initial_hdr_read = FALSE; + + hash_table_clear(uidlist->files, FALSE); + array_clear(&uidlist->records); +} + void maildir_uidlist_deinit(struct maildir_uidlist **_uidlist) { struct maildir_uidlist *uidlist = *_uidlist; @@ -421,7 +436,8 @@ static bool maildir_uidlist_next(struct maildir_uidlist *uidlist, const char *line) { - struct maildir_uidlist_rec *rec, *old_rec; + struct maildir_uidlist_rec *rec, *old_rec, *const *recs; + unsigned int count; uint32_t uid; uid = 0; @@ -486,7 +502,25 @@ } old_rec = hash_table_lookup(uidlist->files, line); - if (old_rec != NULL) { + if (old_rec == NULL) { + /* no conflicts */ + } else if (old_rec->uid == uid) { + /* most likely this is a record we saved ourself, but couldn't + update last_seen_uid because uidlist wasn't refreshed while + it was locked. + + another possibility is a duplicate file record. currently + it would be a bug, but not that big of a deal. also perhaps + in future such duplicate lines could be used to update + extended fields. so just let it through anyway. + + we'll waste a bit of memory here by allocating the record + twice, but that's not really a problem. */ + rec->filename = old_rec->filename; + hash_table_insert(uidlist->files, rec->filename, rec); + uidlist->unsorted = TRUE; + return TRUE; + } else { /* This can happen if expunged file is moved back and the file was appended to uidlist. */ i_warning("%s: Duplicate file entry at line %u: " @@ -501,6 +535,13 @@ uidlist->recreate = TRUE; } + recs = array_get(&uidlist->records, &count); + if (count > 0 && recs[count-1]->uid > uid) { + /* we most likely have some records in the array that we saved + ourself without refreshing uidlist */ + uidlist->unsorted = TRUE; + } + rec->filename = p_strdup(uidlist->record_pool, line); hash_table_insert(uidlist->files, rec->filename, rec); array_append(&uidlist->records, &rec, 1); @@ -592,6 +633,17 @@ return 1; } +static void maildir_uidlist_records_sort_by_uid(struct maildir_uidlist *uidlist) +{ + struct maildir_uidlist_rec **recs; + unsigned int count; + + recs = array_get_modifiable(&uidlist->records, &count); + qsort(recs, count, sizeof(*recs), maildir_uid_cmp); + + uidlist->unsorted = FALSE; +} + static int maildir_uidlist_update_read(struct maildir_uidlist *uidlist, bool *retry_r, bool try_retry) @@ -684,9 +736,13 @@ if (input->stream_errno != 0) ret = -1; + if (uidlist->unsorted) { + uidlist->recreate = TRUE; + maildir_uidlist_records_sort_by_uid(uidlist); + } if (uidlist->next_uid <= uidlist->prev_read_uid) uidlist->next_uid = uidlist->prev_read_uid + 1; - if (uidlist->next_uid < orig_next_uid) { + if (ret > 0 && uidlist->next_uid < orig_next_uid) { mail_storage_set_critical(storage, "%s: next_uid was lowered (%u -> %u)", uidlist->path, orig_next_uid, @@ -716,6 +772,7 @@ mail_storage_set_critical(storage, "read(%s) failed: %m", uidlist->path); } + uidlist->last_read_offset = 0; } i_stream_destroy(&input); @@ -799,8 +856,11 @@ if (uidlist->fd != -1) { ret = maildir_uidlist_has_changed(uidlist, &recreated); - if (ret <= 0) + if (ret <= 0) { + if (UIDLIST_IS_LOCKED(uidlist)) + uidlist->locked_refresh = TRUE; return ret; + } if (recreated) maildir_uidlist_close(uidlist); @@ -1247,9 +1307,9 @@ if (!uidlist->locked_refresh) return FALSE; + if (ctx->finish_change_counter != uidlist->change_counter) return TRUE; - if (uidlist->fd == -1 || uidlist->version != UIDLIST_VERSION) return TRUE; return maildir_uidlist_want_compress(ctx); @@ -1350,7 +1410,7 @@ nonblock = (sync_flags & MAILDIR_UIDLIST_SYNC_TRYLOCK) != 0; refresh = (sync_flags & MAILDIR_UIDLIST_SYNC_NOREFRESH) == 0; - ret = maildir_uidlist_lock_timeout(uidlist, nonblock, refresh); + ret = maildir_uidlist_lock_timeout(uidlist, nonblock, refresh, refresh); if (ret <= 0) { if (ret < 0 || !nonblock) return ret; @@ -1400,6 +1460,7 @@ } return 1; } + i_assert(uidlist->locked_refresh); ctx->record_pool = pool_alloconly_create(MEMPOOL_GROWING "maildir_uidlist_sync", 16384); @@ -1627,10 +1688,11 @@ recs[dest]->flags &= ~MAILDIR_UIDLIST_REC_FLAG_MOVED; } + if (ctx->uidlist->locked_refresh) + ctx->uidlist->last_seen_uid = ctx->uidlist->next_uid-1; + ctx->new_files_count = 0; ctx->first_nouid_pos = (unsigned int)-1; - - ctx->uidlist->last_seen_uid = ctx->uidlist->next_uid-1; ctx->uidlist->change_counter++; ctx->finish_change_counter = ctx->uidlist->change_counter; } @@ -1672,28 +1734,39 @@ if (!ctx->failed) maildir_uidlist_swap(ctx); } else { - if (ctx->new_files_count != 0) + if (ctx->new_files_count != 0 && !ctx->failed) { + i_assert(ctx->changed); + i_assert(ctx->locked); maildir_uidlist_assign_uids(ctx); + } } ctx->finished = TRUE; - ctx->uidlist->initial_sync = TRUE; i_assert(ctx->locked || !ctx->changed); if ((ctx->changed || maildir_uidlist_want_compress(ctx)) && !ctx->failed && ctx->locked) T_BEGIN { - if (maildir_uidlist_sync_update(ctx) < 0) + if (maildir_uidlist_sync_update(ctx) < 0) { + /* we couldn't write everything we wanted. make sure + we don't continue using those UIDs */ + maildir_uidlist_reset(ctx->uidlist); ctx->failed = TRUE; + } } T_END; } -int maildir_uidlist_sync_deinit(struct maildir_uidlist_sync_ctx **_ctx) +int maildir_uidlist_sync_deinit(struct maildir_uidlist_sync_ctx **_ctx, + bool success) { struct maildir_uidlist_sync_ctx *ctx = *_ctx; - int ret = ctx->failed ? -1 : 0; + int ret; *_ctx = NULL; + if (!success) + ctx->failed = TRUE; + ret = ctx->failed ? -1 : 0; + if (!ctx->finished) maildir_uidlist_sync_finish(ctx); if (ctx->partial) diff -r 0aa17f3e4a6d -r 0bb192fe0abd src/lib-storage/index/maildir/maildir-uidlist.h --- a/src/lib-storage/index/maildir/maildir-uidlist.h Mon May 04 16:42:43 2009 -0400 +++ b/src/lib-storage/index/maildir/maildir-uidlist.h Mon May 04 16:43:59 2009 -0400 @@ -109,7 +109,8 @@ maildir_uidlist_sync_get_full_filename(struct maildir_uidlist_sync_ctx *ctx, const char *filename); void maildir_uidlist_sync_finish(struct maildir_uidlist_sync_ctx *ctx); -int maildir_uidlist_sync_deinit(struct maildir_uidlist_sync_ctx **ctx); +int maildir_uidlist_sync_deinit(struct maildir_uidlist_sync_ctx **ctx, + bool success); bool maildir_uidlist_get_uid(struct maildir_uidlist *uidlist, const char *filename, uint32_t *uid_r);