dovecot: Removed explicit locking from views and maps. They were...

dovecot at dovecot.org dovecot at dovecot.org
Sun Aug 12 18:03:05 EEST 2007


details:   http://hg.dovecot.org/dovecot/rev/913b188f4dd4
changeset: 6275:913b188f4dd4
user:      Timo Sirainen <tss at iki.fi>
date:      Sun Aug 12 16:43:05 2007 +0300
description:
Removed explicit locking from views and maps. They were already locked all
the time when they were used. Because of this change several functions can
no longer fail, so they were changed to return void, and a lot of pointless
error handling was removed.

diffstat:

45 files changed, 349 insertions(+), 662 deletions(-)
src/imap/cmd-append.c                              |   17 --
src/imap/cmd-select.c                              |   12 -
src/imap/imap-search.c                             |   11 -
src/imap/imap-status.c                             |    2 
src/imap/imap-sync.c                               |    8 -
src/imap/imap-thread.c                             |    6 
src/lib-index/mail-cache-compress.c                |    7 
src/lib-index/mail-cache-decisions.c               |    7 
src/lib-index/mail-index-map.c                     |   40 -----
src/lib-index/mail-index-private.h                 |    8 -
src/lib-index/mail-index-sync-ext.c                |    4 
src/lib-index/mail-index-sync-keywords.c           |    9 -
src/lib-index/mail-index-sync-update.c             |   13 -
src/lib-index/mail-index-transaction-view.c        |   70 ++++-----
src/lib-index/mail-index-view-private.h            |   21 +-
src/lib-index/mail-index-view-sync.c               |   36 ++--
src/lib-index/mail-index-view.c                    |  145 +++++++-------------
src/lib-index/mail-index.h                         |   27 +--
src/lib-index/mailbox-list-index-sync.c            |   26 +--
src/lib-storage/index/cydir/cydir-sync.c           |   36 +---
src/lib-storage/index/dbox/dbox-mail.c             |    6 
src/lib-storage/index/dbox/dbox-sync-expunge.c     |   53 ++-----
src/lib-storage/index/dbox/dbox-sync-full.c        |   10 -
src/lib-storage/index/dbox/dbox-sync.c             |   16 --
src/lib-storage/index/index-fetch.c                |   14 -
src/lib-storage/index/index-mail.c                 |    5 
src/lib-storage/index/index-search.c               |  145 +++++++-------------
src/lib-storage/index/index-status.c               |   33 +---
src/lib-storage/index/index-storage.c              |    2 
src/lib-storage/index/index-storage.h              |   14 -
src/lib-storage/index/index-sync-changes.c         |   11 -
src/lib-storage/index/index-sync.c                 |   64 ++------
src/lib-storage/index/index-transaction.c          |    1 
src/lib-storage/index/maildir/maildir-sync-index.c |    6 
src/lib-storage/index/maildir/maildir-sync.c       |    8 -
src/lib-storage/index/mbox/mbox-sync-parse.c       |    6 
src/lib-storage/index/mbox/mbox-sync.c             |   14 -
src/lib-storage/list/index-mailbox-list-sync.c     |   19 +-
src/lib-storage/list/index-mailbox-list.c          |    4 
src/lib-storage/mail-storage-private.h             |    8 -
src/lib-storage/mail-storage.c                     |   18 +-
src/lib-storage/mail-storage.h                     |    8 -
src/plugins/fts-squat/fts-backend-squat.c          |    4 
src/plugins/fts/fts-storage.c                      |   29 +---
src/plugins/lazy-expunge/lazy-expunge-plugin.c     |    8 -

diffs (truncated from 2231 to 300 lines):

diff -r bd67afb92ee5 -r 913b188f4dd4 src/imap/cmd-append.c
--- a/src/imap/cmd-append.c	Sun Aug 12 16:34:37 2007 +0300
+++ b/src/imap/cmd-append.c	Sun Aug 12 16:43:05 2007 +0300
@@ -441,7 +441,7 @@ get_mailbox(struct client_command_contex
 	return box;
 }
 
-static int get_keywords(struct cmd_append_context *ctx)
+static void get_keywords(struct cmd_append_context *ctx)
 {
 	struct client *client = ctx->client;
 	struct mailbox_status status;
@@ -450,10 +450,7 @@ static int get_keywords(struct cmd_appen
 	   client_parse_mail_flags()'s keyword verification works.
 	   however if we're not appending to selected mailbox, we'll
 	   need to restore the keywords list. */
-	if (mailbox_get_status(ctx->box, STATUS_KEYWORDS,
-			       &status) < 0)
-		return -1;
-
+	mailbox_get_status(ctx->box, STATUS_KEYWORDS, &status);
 	if (ctx->box != client->mailbox) {
 		ctx->old_keywords = client->keywords;
 
@@ -462,7 +459,6 @@ static int get_keywords(struct cmd_appen
 			pool_alloconly_create("append keywords pool", 256);
 	}
 	client_save_keywords(&client->keywords, status.keywords);
-	return 0;
 }
 
 bool cmd_append(struct client_command_context *cmd)
@@ -487,15 +483,10 @@ bool cmd_append(struct client_command_co
 	else {
 		ctx->storage = mailbox_get_storage(ctx->box);
 
-		if (get_keywords(ctx) < 0) {
-			client_send_storage_error(cmd, ctx->storage);
-			mailbox_close(&ctx->box);
-			ctx->failed = TRUE;
-		} else {
-			ctx->t = mailbox_transaction_begin(ctx->box,
+		get_keywords(ctx);
+		ctx->t = mailbox_transaction_begin(ctx->box,
 					MAILBOX_TRANSACTION_FLAG_EXTERNAL |
 					MAILBOX_TRANSACTION_FLAG_ASSIGN_UIDS);
-		}
 	}
 
 	io_remove(&client->io);
diff -r bd67afb92ee5 -r 913b188f4dd4 src/imap/cmd-select.c
--- a/src/imap/cmd-select.c	Sun Aug 12 16:34:37 2007 +0300
+++ b/src/imap/cmd-select.c	Sun Aug 12 16:43:05 2007 +0300
@@ -42,15 +42,9 @@ bool _cmd_select_full(struct client_comm
 		return TRUE;
 	}
 
-	if (mailbox_get_status(box, STATUS_MESSAGES | STATUS_RECENT |
-			       STATUS_FIRST_UNSEEN_SEQ | STATUS_UIDVALIDITY |
-			       STATUS_UIDNEXT | STATUS_KEYWORDS,
-			       &status) < 0) {
-		client_send_storage_error(cmd, storage);
-		mailbox_close(&box);
-		return TRUE;
-	}
-
+	mailbox_get_status(box, STATUS_MESSAGES | STATUS_RECENT |
+			   STATUS_FIRST_UNSEEN_SEQ | STATUS_UIDVALIDITY |
+			   STATUS_UIDNEXT | STATUS_KEYWORDS, &status);
 	client_save_keywords(&client->keywords, status.keywords);
 	client->messages_count = status.messages;
 	client->recent_count = status.recent;
diff -r bd67afb92ee5 -r 913b188f4dd4 src/imap/imap-search.c
--- a/src/imap/imap-search.c	Sun Aug 12 16:34:37 2007 +0300
+++ b/src/imap/imap-search.c	Sun Aug 12 16:43:05 2007 +0300
@@ -35,15 +35,8 @@ imap_uidset_parse(pool_t pool, struct ma
 		}
 
 		last = seqset->seq2 == (uint32_t)-1;
-		if (mailbox_get_uids(box, seqset->seq1, seqset->seq2,
-				     &seqset->seq1, &seqset->seq2) < 0) {
-			struct mail_storage *storage = mailbox_get_storage(box);
-			enum mail_error error;
-
-			*error_r = mail_storage_get_last_error(storage, &error);
-			return -1;
-		}
-
+		mailbox_get_uids(box, seqset->seq1, seqset->seq2,
+				 &seqset->seq1, &seqset->seq2);
 		if (seqset->seq1 == 0 && last) {
 			/* we need special case for too_high_uid:* case */
 			seqset->seq1 = seqset->seq2 = (uint32_t)-1;
diff -r bd67afb92ee5 -r 913b188f4dd4 src/imap/imap-status.c
--- a/src/imap/imap-status.c	Sun Aug 12 16:34:37 2007 +0300
+++ b/src/imap/imap-status.c	Sun Aug 12 16:43:05 2007 +0300
@@ -68,7 +68,7 @@ bool imap_status_get(struct client *clie
 	}
 
 	if (!failed)
-		failed = mailbox_get_status(box, items, status_r) < 0;
+		mailbox_get_status(box, items, status_r);
 
 	if (box != client->mailbox)
 		mailbox_close(&box);
diff -r bd67afb92ee5 -r 913b188f4dd4 src/imap/imap-sync.c
--- a/src/imap/imap-sync.c	Sun Aug 12 16:34:37 2007 +0300
+++ b/src/imap/imap-sync.c	Sun Aug 12 16:43:05 2007 +0300
@@ -50,11 +50,9 @@ imap_sync_init(struct client *client, st
 	ctx->messages_count = client->messages_count;
 
 	/* if keyword list changed, send the new list before flag changes */
-	if (mailbox_get_status(ctx->box, STATUS_KEYWORDS, &status) == 0) {
-		if (client_save_keywords(&client->keywords, status.keywords))
-			client_send_mailbox_flags(client, box, status.keywords);
-	}
-
+	mailbox_get_status(ctx->box, STATUS_KEYWORDS, &status);
+	if (client_save_keywords(&client->keywords, status.keywords))
+		client_send_mailbox_flags(client, box, status.keywords);
 	return ctx;
 }
 
diff -r bd67afb92ee5 -r 913b188f4dd4 src/imap/imap-thread.c
--- a/src/imap/imap-thread.c	Sun Aug 12 16:34:37 2007 +0300
+++ b/src/imap/imap-thread.c	Sun Aug 12 16:43:05 2007 +0300
@@ -688,10 +688,8 @@ static void gather_base_subjects(struct 
 
 		if (!ctx->id_is_uid)
 			seq = id;
-		else {
-			if (mailbox_get_uids(ctx->box, id, id, &seq, &seq) < 0)
-				seq = 0;
-		}
+		else
+			mailbox_get_uids(ctx->box, id, id, &seq, &seq);
 
 		if (seq != 0 && mail_set_seq(ctx->mail, seq) == 0) {
 			t_push();
diff -r bd67afb92ee5 -r 913b188f4dd4 src/lib-index/mail-cache-compress.c
--- a/src/lib-index/mail-cache-compress.c	Sun Aug 12 16:34:37 2007 +0300
+++ b/src/lib-index/mail-cache-compress.c	Sun Aug 12 16:43:05 2007 +0300
@@ -129,10 +129,9 @@ mail_cache_copy(struct mail_cache *cache
 		first_new_seq = 1;
 		message_count = mail_index_view_get_messages_count(view);
 	} else {
-		if (mail_index_lookup_uid_range(view, idx_hdr->day_first_uid[7],
-						(uint32_t)-1, &first_new_seq,
-						&message_count) < 0)
-			return -1;
+		mail_index_lookup_uid_range(view, idx_hdr->day_first_uid[7],
+					    (uint32_t)-1, &first_new_seq,
+					    &message_count);
 		if (first_new_seq == 0)
 			first_new_seq = message_count+1;
 	}
diff -r bd67afb92ee5 -r 913b188f4dd4 src/lib-index/mail-cache-decisions.c
--- a/src/lib-index/mail-cache-decisions.c	Sun Aug 12 16:34:37 2007 +0300
+++ b/src/lib-index/mail-cache-decisions.c	Sun Aug 12 16:43:05 2007 +0300
@@ -87,8 +87,7 @@ void mail_cache_decision_state_update(st
 	}
 
 	/* see if we want to change decision from TEMP to YES */
-	if (mail_index_lookup_uid(view->view, seq, &uid) < 0)
-		return;
+	mail_index_lookup_uid(view->view, seq, &uid);
 	hdr = mail_index_get_header(view->view);
 
 	if (ioloop_time - cache->fields[field].last_used > 3600*24) {
@@ -136,6 +135,6 @@ void mail_cache_decision_add(struct mail
 	cache->fields[field].decision_dirty = TRUE;
 	cache->field_header_write_pending = TRUE;
 
-	if (mail_index_lookup_uid(view->view, seq, &uid) == 0)
-		cache->fields[field].uid_highwater = uid;
+	mail_index_lookup_uid(view->view, seq, &uid);
+	cache->fields[field].uid_highwater = uid;
 }
diff -r bd67afb92ee5 -r 913b188f4dd4 src/lib-index/mail-index-map.c
--- a/src/lib-index/mail-index-map.c	Sun Aug 12 16:34:37 2007 +0300
+++ b/src/lib-index/mail-index-map.c	Sun Aug 12 16:43:05 2007 +0300
@@ -673,8 +673,6 @@ static int mail_index_map_latest_file(st
 	new_map = mail_index_map_alloc(index);
 	if (use_mmap) {
 		new_map->rec_map->lock_id = lock_id;
-		new_map->rec_map->lock_count++;
-		new_map->locked = TRUE;
 		ret = mail_index_mmap(new_map, file_size);
 	} else {
 		ret = mail_index_read_map(new_map, file_size);
@@ -751,6 +749,9 @@ static void mail_index_record_map_free(s
 static void mail_index_record_map_free(struct mail_index_map *map,
 				       struct mail_index_record_map *rec_map)
 {
+	if (rec_map->lock_id != 0)
+		mail_index_unlock(map->index, &rec_map->lock_id);
+
 	if (rec_map->buffer != NULL) {
 		i_assert(rec_map->mmap_base == NULL);
 		buffer_free(rec_map->buffer);
@@ -791,7 +792,6 @@ void mail_index_unmap(struct mail_index_
 		return;
 
 	i_assert(map->refcount == 0);
-	mail_index_map_unlock(map);
 	mail_index_record_map_unlink(map);
 
 	if (map->extension_pool != NULL)
@@ -800,35 +800,6 @@ void mail_index_unmap(struct mail_index_
 		array_free(&map->keyword_idx_map);
 	buffer_free(map->hdr_copy_buf);
 	i_free(map);
-}
-
-int mail_index_map_lock(struct mail_index_map *map)
-{
-	if (map->locked)
-		return 0;
-
-	if (map->rec_map->lock_id != 0 || MAIL_INDEX_MAP_IS_IN_MEMORY(map)) {
-		map->rec_map->lock_count++;
-		map->locked = TRUE;
-		return 0;
-	}
-
-	if (mail_index_lock_shared(map->index, &map->rec_map->lock_id) < 0)
-		return -1;
-
-	mail_index_map_copy_hdr(map, map->rec_map->mmap_base);
-	map->rec_map->lock_count++;
-	map->locked = TRUE;
-	return 0;
-}
-
-void mail_index_map_unlock(struct mail_index_map *map)
-{
-	if (!map->locked)
-		return;
-
-	if (--map->rec_map->lock_count == 0)
-		mail_index_unlock(map->index, &map->rec_map->lock_id);
 }
 
 static void mail_index_map_copy_records(struct mail_index_record_map *dest,
@@ -946,7 +917,6 @@ void mail_index_record_map_move_to_priva
 	mail_index_map_copy_records(new_map, map->rec_map,
 				    map->hdr.record_size);
 
-	mail_index_map_unlock(map);
 	mail_index_record_map_unlink(map);
 	map->rec_map = new_map;
 }
@@ -967,11 +937,11 @@ void mail_index_map_move_to_memory(struc
 				    map->hdr.record_size);
 	mail_index_map_copy_header(map, map);
 
-	mail_index_map_unlock(map);
 	if (new_map != map->rec_map) {
 		mail_index_record_map_unlink(map);
 		map->rec_map = new_map;
-	} else if (new_map->mmap_base != NULL) {
+	} else {
+		mail_index_unlock(map->index, &new_map->lock_id);
 		if (munmap(new_map->mmap_base, new_map->mmap_size) < 0)
 			mail_index_set_syscall_error(map->index, "munmap()");
 		new_map->mmap_base = NULL;
diff -r bd67afb92ee5 -r 913b188f4dd4 src/lib-index/mail-index-private.h
--- a/src/lib-index/mail-index-private.h	Sun Aug 12 16:34:37 2007 +0300
+++ b/src/lib-index/mail-index-private.h	Sun Aug 12 16:43:05 2007 +0300
@@ -108,8 +108,6 @@ struct mail_index_record_map {
 
 	void *mmap_base;
 	size_t mmap_size, mmap_used_size;
-
-	unsigned int lock_count;
 	unsigned int lock_id;
 
 	buffer_t *buffer;
@@ -141,7 +139,6 @@ struct mail_index_map {
 
 	struct mail_index_record_map *rec_map;
 
-	unsigned int locked:1;
 	unsigned int keywords_read:1;
 	unsigned int write_base_header:1;
 	unsigned int write_ext_header:1;
@@ -273,11 +270,6 @@ int mail_index_map(struct mail_index *in
 /* Unreference given mapping and unmap it if it's dropped to zero. */
 void mail_index_unmap(struct mail_index_map **map);
 
-/* Lock the map if the data is mmaped and map is unlocked. */
-int mail_index_map_lock(struct mail_index_map *map);
-/* Unlock the map if it's locked. */
-void mail_index_map_unlock(struct mail_index_map *map);
-
 /* Clone a map. The returned map is always in memory. */
 struct mail_index_map *mail_index_map_clone(const struct mail_index_map *map);
 void mail_index_record_map_move_to_private(struct mail_index_map *map);
diff -r bd67afb92ee5 -r 913b188f4dd4 src/lib-index/mail-index-sync-ext.c
--- a/src/lib-index/mail-index-sync-ext.c	Sun Aug 12 16:34:37 2007 +0300


More information about the dovecot-cvs mailing list