dovecot-2.0: maildir_copy_with_hardlinks=yes no longer has a rac...

dovecot at dovecot.org dovecot at dovecot.org
Thu Feb 18 06:18:42 EET 2010


details:   http://hg.dovecot.org/dovecot-2.0/rev/502d6048a281
changeset: 10741:502d6048a281
user:      Timo Sirainen <tss at iki.fi>
date:      Thu Feb 18 06:15:47 2010 +0200
description:
maildir_copy_with_hardlinks=yes no longer has a race condition.

diffstat:

 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(-)

diffs (123 lines):

diff -r 386b13dfee04 -r 502d6048a281 src/lib-storage/index/maildir/maildir-copy.c
--- a/src/lib-storage/index/maildir/maildir-copy.c	Thu Feb 18 05:53:59 2010 +0200
+++ b/src/lib-storage/index/maildir/maildir-copy.c	Thu Feb 18 06:15:47 2010 +0200
@@ -125,27 +125,15 @@
 
 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 @@
 
 	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 @@
 	}
 
 	/* 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 -r 386b13dfee04 -r 502d6048a281 src/lib-storage/index/maildir/maildir-save.c
--- a/src/lib-storage/index/maildir/maildir-save.c	Thu Feb 18 05:53:59 2010 +0200
+++ b/src/lib-storage/index/maildir/maildir-save.c	Thu Feb 18 06:15:47 2010 +0200
@@ -66,6 +66,7 @@
 	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 @@
 }
 
 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 @@
 		 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 @@
 	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 @@
 				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 @@
 		/* 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 -r 386b13dfee04 -r 502d6048a281 src/lib-storage/index/maildir/maildir-storage.h
--- a/src/lib-storage/index/maildir/maildir-storage.h	Thu Feb 18 05:53:59 2010 +0200
+++ b/src/lib-storage/index/maildir/maildir-storage.h	Thu Feb 18 06:15:47 2010 +0200
@@ -122,7 +122,8 @@
 			       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);
 


More information about the dovecot-cvs mailing list