# HG changeset patch # User Timo Sirainen # Date 1124046174 -10800 # Node ID e9695ec7925b596faacaf81cdd70e49b5c061138 # Parent e963246c0981462c552e4c898730306019f8c2ae Recursive maildir uidlist syncs caused assert crashes. Also did some cleanups. diff -r e963246c0981 -r e9695ec7925b src/lib-storage/index/maildir/maildir-save.c --- a/src/lib-storage/index/maildir/maildir-save.c Sun Aug 14 21:25:22 2005 +0300 +++ b/src/lib-storage/index/maildir/maildir-save.c Sun Aug 14 22:02:54 2005 +0300 @@ -361,8 +361,8 @@ return -1; } - ret = maildir_uidlist_lock(ctx->mbox->uidlist); - if (ret <= 0) { + if (maildir_uidlist_sync_init(ctx->mbox->uidlist, TRUE, + &ctx->uidlist_sync_ctx) <= 0) { /* error or timeout - our transaction is broken */ maildir_sync_index_abort(ctx->sync_ctx); maildir_save_commit_abort(ctx, ctx->files); @@ -378,9 +378,6 @@ MAILDIR_UIDLIST_REC_FLAG_RECENT; /* move them into new/ */ - ctx->uidlist_sync_ctx = - maildir_uidlist_sync_init(ctx->mbox->uidlist, TRUE); - ret = 0; for (mf = ctx->files; mf != NULL; mf = mf->next) { t_push(); diff -r e963246c0981 -r e9695ec7925b src/lib-storage/index/maildir/maildir-sync.c --- a/src/lib-storage/index/maildir/maildir-sync.c Sun Aug 14 21:25:22 2005 +0300 +++ b/src/lib-storage/index/maildir/maildir-sync.c Sun Aug 14 22:02:54 2005 +0300 @@ -1222,17 +1222,16 @@ return -1; } - if ((ret = maildir_uidlist_lock(ctx->mbox->uidlist)) <= 0) { + ctx->partial = !cur_changed; + ret = maildir_uidlist_sync_init(ctx->mbox->uidlist, ctx->partial, + &ctx->uidlist_sync_ctx); + if (ret <= 0) { /* failure / timeout. if forced is TRUE, we could still go forward and check only for renamed files, but is it worth the trouble? .. */ return ret; } - ctx->partial = !cur_changed; - ctx->uidlist_sync_ctx = - maildir_uidlist_sync_init(ctx->mbox->uidlist, ctx->partial); - if (new_changed || cur_changed) { /* if we're going to check cur/ dir our current logic requires that new/ dir is checked as well. it's a good idea anyway. */ @@ -1254,6 +1253,10 @@ } if (!ctx->mbox->syncing_commit) { + /* NOTE: index syncing here might cause a re-sync due to + files getting lost, so this function might be called + re-entrantly. FIXME: and that breaks in + maildir_uidlist_sync_deinit() */ ret = maildir_sync_index_finish(ctx->index_sync_ctx, ctx->partial); if (ret < 0) { diff -r e963246c0981 -r e9695ec7925b src/lib-storage/index/maildir/maildir-uidlist.c --- a/src/lib-storage/index/maildir/maildir-uidlist.c Sun Aug 14 21:25:22 2005 +0300 +++ b/src/lib-storage/index/maildir/maildir-uidlist.c Sun Aug 14 22:02:54 2005 +0300 @@ -49,6 +49,8 @@ unsigned int initial_read:1; unsigned int initial_sync:1; + + unsigned int need_rewrite:1; }; struct maildir_uidlist_sync_ctx { @@ -577,20 +579,24 @@ } } -struct maildir_uidlist_sync_ctx * -maildir_uidlist_sync_init(struct maildir_uidlist *uidlist, int partial) +int maildir_uidlist_sync_init(struct maildir_uidlist *uidlist, int partial, + struct maildir_uidlist_sync_ctx **sync_ctx_r) { struct maildir_uidlist_sync_ctx *ctx; size_t size; + int ret; - ctx = i_new(struct maildir_uidlist_sync_ctx, 1); + if ((ret = maildir_uidlist_lock(uidlist)) <= 0) + return ret; + + *sync_ctx_r = ctx = i_new(struct maildir_uidlist_sync_ctx, 1); ctx->uidlist = uidlist; ctx->partial = partial; if (partial) { /* initially mark all nonsynced */ maildir_uidlist_mark_all(uidlist, TRUE); - return ctx; + return 1; } ctx->record_pool = @@ -600,7 +606,7 @@ size = buffer_get_used_size(uidlist->record_buf); ctx->record_buf = buffer_create_dynamic(default_pool, size); - return ctx; + return 1; } static int @@ -825,17 +831,26 @@ if (!ctx->finished) maildir_uidlist_sync_finish(ctx); - if (ctx->new_files_count != 0 && !ctx->failed) { - t_push(); - ret = maildir_uidlist_rewrite(ctx->uidlist); - t_pop(); - } - if (ctx->partial) maildir_uidlist_mark_all(ctx->uidlist, FALSE); - if (UIDLIST_IS_LOCKED(ctx->uidlist)) - maildir_uidlist_unlock(ctx->uidlist); + if (ctx->uidlist->need_rewrite || + (ctx->new_files_count != 0 && !ctx->failed)) { + if (ctx->uidlist->lock_count > 1) { + /* recursive sync. let the root syncing do + the rewrite */ + ctx->uidlist->need_rewrite = TRUE; + } else { + t_push(); + ret = maildir_uidlist_rewrite(ctx->uidlist); + t_pop(); + + if (ret == 0) + ctx->uidlist->need_rewrite = FALSE; + } + } + + maildir_uidlist_unlock(ctx->uidlist); if (ctx->files != NULL) hash_destroy(ctx->files); diff -r e963246c0981 -r e9695ec7925b src/lib-storage/index/maildir/maildir-uidlist.h --- a/src/lib-storage/index/maildir/maildir-uidlist.h Sun Aug 14 21:25:22 2005 +0300 +++ b/src/lib-storage/index/maildir/maildir-uidlist.h Sun Aug 14 22:02:54 2005 +0300 @@ -3,6 +3,8 @@ #define MAILDIR_UIDLIST_NAME "dovecot-uidlist" +struct maildir_uidlist_sync_ctx; + enum maildir_uidlist_rec_flag { MAILDIR_UIDLIST_REC_FLAG_NEW_DIR = 0x01, MAILDIR_UIDLIST_REC_FLAG_MOVED = 0x02, @@ -37,9 +39,10 @@ void maildir_uidlist_set_uid_validity(struct maildir_uidlist *uidlist, uint32_t uid_validity); -/* Sync uidlist with what's actually on maildir. */ -struct maildir_uidlist_sync_ctx * -maildir_uidlist_sync_init(struct maildir_uidlist *uidlist, int partial); +/* Sync uidlist with what's actually on maildir. Returns same as + maildir_uidlist_lock(). */ +int maildir_uidlist_sync_init(struct maildir_uidlist *uidlist, int partial, + struct maildir_uidlist_sync_ctx **sync_ctx_r); /* Returns 1 = ok, -1 = error, 0 = new file and dovecot-uidlist is locked */ int maildir_uidlist_sync_next_pre(struct maildir_uidlist_sync_ctx *ctx, const char *filename);