dovecot: Don't use uidlist lock to update uidlist. Have a separa...

dovecot at dovecot.org dovecot at dovecot.org
Mon Jul 9 05:44:42 EEST 2007


details:   http://hg.dovecot.org/dovecot/rev/525b449313a7
changeset: 5917:525b449313a7
user:      Timo Sirainen <tss at iki.fi>
date:      Sun Jul 08 23:15:45 2007 +0300
description:
Don't use uidlist lock to update uidlist. Have a separate real dotlock for
locking and create a .tmp file when we need to rewrite the uidlist. This
makes the code cleaner and makes it possible to easily detect stale lock
files.

diffstat:

1 file changed, 122 insertions(+), 178 deletions(-)
src/lib-storage/index/maildir/maildir-uidlist.c |  300 +++++++++--------------

diffs (truncated from 528 to 300 lines):

diff -r d30fa36505fb -r 525b449313a7 src/lib-storage/index/maildir/maildir-uidlist.c
--- a/src/lib-storage/index/maildir/maildir-uidlist.c	Sun Jul 08 23:04:02 2007 +0300
+++ b/src/lib-storage/index/maildir/maildir-uidlist.c	Sun Jul 08 23:15:45 2007 +0300
@@ -5,6 +5,7 @@
 #include "array.h"
 #include "hash.h"
 #include "istream.h"
+#include "ostream.h"
 #include "str.h"
 #include "file-dotlock.h"
 #include "close-keep-errno.h"
@@ -38,32 +39,28 @@ ARRAY_DEFINE_TYPE(maildir_uidlist_rec_p,
 
 struct maildir_uidlist {
 	struct maildir_mailbox *mbox;
-	char *fname;
+	char *path;
 
 	int fd;
 	dev_t fd_dev;
 	ino_t fd_ino;
 
-	int lock_fd;
 	unsigned int lock_count;
+
+	struct dotlock_settings dotlock_settings;
+	struct dotlock *dotlock;
 
 	pool_t record_pool;
 	ARRAY_TYPE(maildir_uidlist_rec_p) records;
 	struct hash_table *files;
 	unsigned int change_counter;
 
-	struct dotlock_settings dotlock_settings;
-	struct dotlock *dotlock;
-
 	unsigned int version;
 	unsigned int uid_validity, next_uid, prev_read_uid, last_seen_uid;
 	uint32_t first_recent_uid;
 
 	unsigned int initial_read:1;
 	unsigned int initial_sync:1;
-
-	unsigned int need_rewrite:1;
-	unsigned int delayed_rewrite:1;
 };
 
 struct maildir_uidlist_sync_ctx {
@@ -96,7 +93,7 @@ static int maildir_uidlist_lock_timeout(
 	struct maildir_mailbox *mbox = uidlist->mbox;
 	const char *path;
 	mode_t old_mask;
-	int fd;
+	int ret;
 
 	if (uidlist->lock_count > 0) {
 		uidlist->lock_count++;
@@ -105,12 +102,12 @@ static int maildir_uidlist_lock_timeout(
 
 	path = t_strconcat(mbox->control_dir, "/" MAILDIR_UIDLIST_NAME, NULL);
         old_mask = umask(0777 & ~mbox->mail_create_mode);
-	fd = file_dotlock_open(&uidlist->dotlock_settings, path,
-			       nonblock ? DOTLOCK_CREATE_FLAG_NONBLOCK : 0,
-			       &uidlist->dotlock);
+	ret = file_dotlock_create(&uidlist->dotlock_settings, path,
+				  nonblock ? DOTLOCK_CREATE_FLAG_NONBLOCK : 0,
+				  &uidlist->dotlock);
 	umask(old_mask);
-	if (fd == -1) {
-		if (errno == EAGAIN) {
+	if (ret <= 0) {
+		if (ret == 0) {
 			mail_storage_set_error(&mbox->storage->storage,
 				MAIL_ERROR_TEMP, MAIL_ERRSTR_LOCK_TIMEOUT);
 			return 0;
@@ -119,20 +116,14 @@ static int maildir_uidlist_lock_timeout(
 			"file_dotlock_open(%s) failed: %m", path);
 		return -1;
 	}
-	uidlist->lock_fd = fd;
-
-	if (mbox->mail_create_gid != (gid_t)-1) {
-		if (fchown(fd, (uid_t)-1, mbox->mail_create_gid) < 0) {
-			mail_storage_set_critical(&mbox->storage->storage,
-				"fchown(%s) failed: %m", path);
-		}
-	}
-
-	/* our view of uidlist must be up-to-date if we plan on changing it */
-	if (maildir_uidlist_update(uidlist) < 0)
+
+	uidlist->lock_count++;
+
+	/* make sure we have the latest changes before changing anything */
+	if (maildir_uidlist_update(uidlist) < 0) {
+		maildir_uidlist_unlock(uidlist);
 		return -1;
-
-	uidlist->lock_count++;
+	}
 	return 1;
 }
 
@@ -165,21 +156,7 @@ void maildir_uidlist_unlock(struct maild
 	if (--uidlist->lock_count > 0)
 		return;
 
-	if (!uidlist->delayed_rewrite) {
-		(void)file_dotlock_delete(&uidlist->dotlock);
-	} else {
-		if (file_dotlock_replace(&uidlist->dotlock, 0) <= 0) {
-			const char *db_path;
-
-			db_path = t_strconcat(uidlist->mbox->control_dir,
-					      "/" MAILDIR_UIDLIST_NAME, NULL);
-			mail_storage_set_critical(
-				&uidlist->mbox->storage->storage,
-				"file_dotlock_replace(%s) failed: %m", db_path);
-		}
-		uidlist->delayed_rewrite = FALSE;
-	}
-	uidlist->lock_fd = -1;
+	(void)file_dotlock_delete(&uidlist->dotlock);
 }
 
 struct maildir_uidlist *maildir_uidlist_init(struct maildir_mailbox *mbox)
@@ -189,9 +166,8 @@ struct maildir_uidlist *maildir_uidlist_
 	uidlist = i_new(struct maildir_uidlist, 1);
 	uidlist->fd = -1;
 	uidlist->mbox = mbox;
-	uidlist->fname =
+	uidlist->path =
 		i_strconcat(mbox->control_dir, "/" MAILDIR_UIDLIST_NAME, NULL);
-	uidlist->lock_fd = -1;
 	i_array_init(&uidlist->records, 128);
 	uidlist->files = hash_create(default_pool, default_pool, 4096,
 				     maildir_filename_base_hash,
@@ -212,7 +188,7 @@ static void maildir_uidlist_close(struct
 {
 	if (uidlist->fd != -1) {
 		if (close(uidlist->fd) < 0)
-			i_error("close(%s) failed: %m", uidlist->fname);
+			i_error("close(%s) failed: %m", uidlist->path);
 		uidlist->fd = -1;
 		uidlist->fd_ino = 0;
 	}
@@ -229,7 +205,7 @@ void maildir_uidlist_deinit(struct maild
 		pool_unref(uidlist->record_pool);
 
 	array_free(&uidlist->records);
-	i_free(uidlist->fname);
+	i_free(uidlist->path);
 	i_free(uidlist);
 }
 
@@ -256,13 +232,13 @@ static int maildir_uidlist_next(struct m
 	if (uid == 0 || *line != ' ') {
 		/* invalid file */
                 mail_storage_set_critical(&uidlist->mbox->storage->storage,
-			"Invalid data in file %s", uidlist->fname);
+			"Invalid data in file %s", uidlist->path);
 		return 0;
 	}
 	if (uid <= uidlist->prev_read_uid) {
                 mail_storage_set_critical(&uidlist->mbox->storage->storage,
 			"UIDs not ordered in file %s (%u > %u)",
-			uidlist->fname, uid, uidlist->prev_read_uid);
+			uidlist->path, uid, uidlist->prev_read_uid);
 		return 0;
 	}
 	uidlist->prev_read_uid = uid;
@@ -276,7 +252,7 @@ static int maildir_uidlist_next(struct m
 	if (uid >= uidlist->next_uid) {
                 mail_storage_set_critical(&uidlist->mbox->storage->storage,
 			"UID larger than next_uid in file %s (%u >= %u)",
-			uidlist->fname, uid, uidlist->next_uid);
+			uidlist->path, uid, uidlist->next_uid);
 		return 0;
 	}
 
@@ -291,7 +267,7 @@ static int maildir_uidlist_next(struct m
 	if (hash_lookup_full(uidlist->files, line, NULL, NULL)) {
                 mail_storage_set_critical(&uidlist->mbox->storage->storage,
 			"Duplicate file in uidlist file %s: %s",
-			uidlist->fname, line);
+			uidlist->path, line);
 		return 0;
 	}
 
@@ -319,11 +295,11 @@ maildir_uidlist_update_read(struct maild
 
 	maildir_uidlist_close(uidlist);
 
-	fd = nfs_safe_open(uidlist->fname, O_RDONLY);
+	fd = nfs_safe_open(uidlist->path, O_RDONLY);
 	if (fd == -1) {
 		if (errno != ENOENT) {
 			mail_storage_set_critical(storage,
-				"open(%s) failed: %m", uidlist->fname);
+				"open(%s) failed: %m", uidlist->path);
 			return -1;
 		}
 		return 0;
@@ -336,7 +312,7 @@ maildir_uidlist_update_read(struct maild
                         return -1;
                 }
                 mail_storage_set_critical(storage,
-			"fstat(%s) failed: %m", uidlist->fname);
+			"fstat(%s) failed: %m", uidlist->path);
 		return -1;
 	}
 
@@ -363,18 +339,18 @@ maildir_uidlist_update_read(struct maild
                 /* broken file */
                 mail_storage_set_critical(storage,
 			"Corrupted header in file %s (version = %u)",
-			uidlist->fname, uidlist->version);
+			uidlist->path, uidlist->version);
 		ret = 0;
 	} else if (uid_validity == uidlist->uid_validity &&
 		   next_uid < uidlist->next_uid) {
                 mail_storage_set_critical(storage,
 			"%s: next_uid was lowered (%u -> %u)",
-			uidlist->fname, uidlist->next_uid, next_uid);
+			uidlist->path, uidlist->next_uid, next_uid);
 		ret = 0;
 	} else if (uid_validity == 0 || next_uid == 0) {
                 mail_storage_set_critical(storage,
 			"%s: Broken header (uidvalidity = %u, next_uid=%u)",
-			uidlist->fname, uid_validity, next_uid);
+			uidlist->path, uid_validity, next_uid);
 		ret = 0;
 	} else {
 		uidlist->uid_validity = uid_validity;
@@ -395,7 +371,7 @@ maildir_uidlist_update_read(struct maild
 
         if (ret == 0) {
                 /* file is broken */
-                (void)unlink(uidlist->fname);
+                (void)unlink(uidlist->path);
         } else if (ret > 0) {
                 /* success */
 		uidlist->fd = fd;
@@ -408,14 +384,14 @@ maildir_uidlist_update_read(struct maild
 		else {
 			errno = input->stream_errno;
 			mail_storage_set_critical(storage,
-				"read(%s) failed: %m", uidlist->fname);
+				"read(%s) failed: %m", uidlist->path);
 		}
 	}
 
 	i_stream_destroy(&input);
 	if (ret <= 0) {
 		if (close(fd) < 0)
-			i_error("close(%s) failed: %m", uidlist->fname);
+			i_error("close(%s) failed: %m", uidlist->path);
 	}
 	return ret;
 }
@@ -429,10 +405,10 @@ int maildir_uidlist_update(struct maildi
         int ret;
 
 	if (uidlist->fd != -1) {
-		if (nfs_safe_stat(uidlist->fname, &st) < 0) {
+		if (nfs_safe_stat(uidlist->path, &st) < 0) {
 			if (errno != ENOENT) {
 				mail_storage_set_critical(storage,
-					"stat(%s) failed: %m", uidlist->fname);
+					"stat(%s) failed: %m", uidlist->path);
 				return -1;
 			}
 			return 0;
@@ -580,32 +556,20 @@ uint32_t maildir_uidlist_get_next_uid(st
 	return !uidlist->initial_read ? 0 : uidlist->next_uid;
 }
 
-static int maildir_uidlist_rewrite_fd(struct maildir_uidlist *uidlist,
-				      const char *temp_path)
+static int maildir_uidlist_rewrite_fd(struct maildir_uidlist *uidlist, int fd,
+				      const char *path)
 {
 	struct mail_storage *storage = &uidlist->mbox->storage->storage;
 	struct maildir_uidlist_iter_ctx *iter;
+	struct ostream *output;
 	string_t *str;
 	uint32_t uid;
         enum maildir_uidlist_rec_flag flags;
 	const char *filename;
-	int ret = 0;
-
-	if (uidlist->delayed_rewrite) {
-		/* already written, truncate */
-		if (lseek(uidlist->lock_fd, 0, SEEK_SET) < 0) {
-			mail_storage_set_critical(storage,
-				"lseek(%s) failed: %m", temp_path);
-			return -1;
-		}
-		if (ftruncate(uidlist->lock_fd, 0) < 0) {
-			mail_storage_set_critical(storage,


More information about the dovecot-cvs mailing list