From ea4c984d8097f24aa18ed5bb364ebaf3b01bc2a3 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Thu, 18 Feb 2010 06:15:47 +0200 Subject: [PATCH] maildir_copy_with_hardlinks=yes no longer has a race condition. --HG-- branch : HEAD --- src/lib-storage/index/maildir/maildir-copy.c | 16 ++-------------- src/lib-storage/index/maildir/maildir-save.c | 16 +++++++++++----- src/lib-storage/index/maildir/maildir-storage.h | 3 ++- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/lib-storage/index/maildir/maildir-copy.c b/src/lib-storage/index/maildir/maildir-copy.c index 179e1b6dbf..34e6dd6987 100644 --- a/src/lib-storage/index/maildir/maildir-copy.c +++ b/src/lib-storage/index/maildir/maildir-copy.c @@ -125,27 +125,15 @@ static int do_hardlink(struct maildir_mailbox *mbox, const char *path, static const char * maildir_copy_get_preserved_fname(struct maildir_mailbox *src_mbox, - struct maildir_mailbox *dest_mbox, uint32_t uid) { enum maildir_uidlist_rec_flag flags; const char *fname; - /* see if the filename exists in destination maildir's - uidlist. if it doesn't, we can use it. otherwise generate - a new filename. FIXME: There's a race condition here if - another process is just doing the same copy. */ if (maildir_uidlist_lookup(src_mbox->uidlist, uid, &flags, &fname) <= 0) return NULL; - if (maildir_uidlist_refresh(dest_mbox->uidlist) <= 0) - return NULL; - if (maildir_uidlist_get_full_filename(dest_mbox->uidlist, - fname) != NULL) { - /* already exists in destination */ - return NULL; - } /* fname may be freed by a later uidlist sync. make sure it gets strduped. */ return t_strcut(t_strdup(fname), ':'); @@ -175,7 +163,7 @@ maildir_copy_hardlink(struct mail_save_context *ctx, struct mail *mail) if (dest_mbox->storage->set->maildir_copy_preserve_filename && src_mbox != NULL) { - filename = maildir_copy_get_preserved_fname(src_mbox, dest_mbox, + filename = maildir_copy_get_preserved_fname(src_mbox, mail->uid); } if (filename == NULL) { @@ -212,7 +200,7 @@ maildir_copy_hardlink(struct mail_save_context *ctx, struct mail *mail) } /* hardlinked to tmp/, treat as normal copied mail */ - maildir_save_add(ctx, do_ctx.dest_fname); + maildir_save_add(ctx, do_ctx.dest_fname, do_ctx.preserve_filename); return 1; } diff --git a/src/lib-storage/index/maildir/maildir-save.c b/src/lib-storage/index/maildir/maildir-save.c index 35f6b429c2..93658c59de 100644 --- a/src/lib-storage/index/maildir/maildir-save.c +++ b/src/lib-storage/index/maildir/maildir-save.c @@ -66,6 +66,7 @@ struct maildir_save_context { uint32_t first_seq, seq, last_nonrecent_uid; unsigned int have_keywords:1; + unsigned int have_preserved_filenames:1; unsigned int locked:1; unsigned int failed:1; unsigned int last_save_finished:1; @@ -140,7 +141,8 @@ maildir_save_transaction_init(struct mailbox_transaction_context *t) } struct maildir_filename * -maildir_save_add(struct mail_save_context *_ctx, const char *base_fname) +maildir_save_add(struct mail_save_context *_ctx, const char *base_fname, + bool preserve_filename) { struct maildir_save_context *ctx = (struct maildir_save_context *)_ctx; struct maildir_filename *mf; @@ -155,7 +157,6 @@ maildir_save_add(struct mail_save_context *_ctx, const char *base_fname) ctx->last_nonrecent_uid < _ctx->uid) ctx->last_nonrecent_uid = _ctx->uid; - /* now, we want to be able to rollback the whole append session, so we'll just store the name of this temp file and move it later into new/ or cur/. */ @@ -167,6 +168,9 @@ maildir_save_add(struct mail_save_context *_ctx, const char *base_fname) mf->flags = _ctx->flags; mf->size = (uoff_t)-1; mf->vsize = (uoff_t)-1; + mf->preserve_filename = preserve_filename; + if (preserve_filename) + ctx->have_preserved_filenames = TRUE; ctx->file_last = mf; i_assert(*ctx->files_tail == NULL); @@ -394,9 +398,7 @@ int maildir_save_begin(struct mail_save_context *_ctx, struct istream *input) ctx->input = i_stream_create_crlf(input); else ctx->input = i_stream_create_lf(input); - mf = maildir_save_add(_ctx, fname); - if (fname == _ctx->guid) - mf->preserve_filename = TRUE; + mf = maildir_save_add(_ctx, fname, fname == _ctx->guid); } } T_END; @@ -912,6 +914,10 @@ int maildir_transaction_save_commit_pre(struct mail_save_context *_ctx) /* we want to assign UIDs, we must lock uidlist */ } else if (ctx->have_keywords) { /* keywords file updating relies on uidlist lock. */ + } else if (ctx->have_preserved_filenames) { + /* we're trying to use some potentially existing filenames. + we must lock to avoid race conditions where two sessions + try to save the same filename. */ } else { /* no requirement to lock uidlist. if we happen to get a lock, assign uids. */ diff --git a/src/lib-storage/index/maildir/maildir-storage.h b/src/lib-storage/index/maildir/maildir-storage.h index 71bc3a7379..022ac64fdc 100644 --- a/src/lib-storage/index/maildir/maildir-storage.h +++ b/src/lib-storage/index/maildir/maildir-storage.h @@ -122,7 +122,8 @@ void maildir_save_add_conflict(struct mailbox_transaction_context *t, uint32_t old_uid, uint32_t new_uid); struct maildir_filename * -maildir_save_add(struct mail_save_context *_ctx, const char *base_fname); +maildir_save_add(struct mail_save_context *_ctx, const char *base_fname, + bool preserve_filename); const char *maildir_save_file_get_path(struct mailbox_transaction_context *t, uint32_t seq); -- 2.47.3