]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
maildir_copy_with_hardlinks=yes no longer has a race condition.
authorTimo Sirainen <tss@iki.fi>
Thu, 18 Feb 2010 04:15:47 +0000 (06:15 +0200)
committerTimo Sirainen <tss@iki.fi>
Thu, 18 Feb 2010 04:15:47 +0000 (06:15 +0200)
--HG--
branch : HEAD

src/lib-storage/index/maildir/maildir-copy.c
src/lib-storage/index/maildir/maildir-save.c
src/lib-storage/index/maildir/maildir-storage.h

index 179e1b6dbfe120243cd2e508bb9ecfb58307b854..34e6dd6987900b987a89e473269248bc51d81b89 100644 (file)
@@ -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;
 }
 
index 35f6b429c21c4717879bec6ca76df9cd5a0dae7d..93658c59debf912a07c05366417bc965caf5d4aa 100644 (file)
@@ -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. */
index 71bc3a737972f65814c7fac0e78328c995bbaeb2..022ac64fdcbf8476ea7f1f630d3ab103f0e1c7c8 100644 (file)
@@ -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);