Mercurial > dovecot > core-2.2

changeset 7632:6e2e4e5c52f3 HEAD

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fixed CLOSE HIGEHSTMODSEQ race condition. Added some checks to make sure mailbox isn't being closed/opened while the previous SELECT/CLOSE is still being processed.
author Timo Sirainen <tss@iki.fi>
date Sun, 16 Mar 2008 07:18:06 +0200
parents 5d3b6a766032
children 27a0de620b0c
files src/imap/client.c src/imap/client.h src/imap/cmd-close.c src/imap/cmd-expunge.c src/imap/cmd-select.c src/imap/cmd-unselect.c src/imap/imap-expunge.c src/imap/imap-expunge.h
diffstat 8 files changed, 69 insertions(+), 44 deletions(-) [+]
line wrap: on
line diff
--- a/src/imap/client.c	Sun Mar 16 06:53:58 2008 +0200
+++ b/src/imap/client.c	Sun Mar 16 07:18:06 2008 +0200
@@ -364,6 +364,11 @@
 			/* don't do anything until syncing is finished */
 			return TRUE;
 		}
+		if (cmd->client->changing_mailbox) {
+			/* don't do anything until mailbox is fully
+			   opened/closed */
+			return TRUE;
+		}
 		return FALSE;
 	}
 
--- a/src/imap/client.h	Sun Mar 16 06:53:58 2008 +0200
+++ b/src/imap/client.h	Sun Mar 16 07:18:06 2008 +0200
@@ -92,7 +92,7 @@
 	unsigned int destroyed:1;
 	unsigned int handling_input:1;
 	unsigned int syncing:1;
-	unsigned int selecting:1;
+	unsigned int changing_mailbox:1;
 	unsigned int input_skip_line:1; /* skip all the data until we've
 					   found a new line */
 };
--- a/src/imap/cmd-close.c	Sun Mar 16 06:53:58 2008 +0200
+++ b/src/imap/cmd-close.c	Sun Mar 16 07:18:06 2008 +0200
@@ -4,39 +4,60 @@
 #include "commands.h"
 #include "imap-expunge.h"
 
+static void cmd_close_finish(struct client *client)
+{
+	if (mailbox_close(&client->mailbox) < 0) {
+		client_send_untagged_storage_error(client,
+			mailbox_get_storage(client->mailbox));
+	}
+	client_update_mailbox_flags(client, NULL);
+	client->changing_mailbox = FALSE;
+}
+
+static bool cmd_close_callback(struct client_command_context *cmd)
+{
+	struct mailbox_status status;
+
+	mailbox_get_status(cmd->client->mailbox,
+			   STATUS_HIGHESTMODSEQ, &status);
+	cmd_close_finish(cmd->client);
+	client_send_tagline(cmd, t_strdup_printf(
+		"OK [HIGHESTMODSEQ %llu] Close completed.",
+		(unsigned long long)status.highest_modseq));
+	return TRUE;
+}
+
 bool cmd_close(struct client_command_context *cmd)
 {
 	struct client *client = cmd->client;
 	struct mailbox *mailbox = client->mailbox;
 	struct mail_storage *storage;
-	struct mailbox_status status;
-	bool show_highestmodseq;
+	int ret;
 
 	if (!client_verify_open_mailbox(cmd))
 		return TRUE;
 
-	storage = mailbox_get_storage(mailbox);
-	client->mailbox = NULL;
+	i_assert(!client->changing_mailbox);
+	client->changing_mailbox = TRUE;
 
-	show_highestmodseq =
-		(cmd->client->enabled_features & MAILBOX_FEATURE_QRESYNC) != 0;
-
-	if (!imap_expunge(mailbox, NULL))
-		client_send_untagged_storage_error(client, storage);
-	else if (mailbox_sync(mailbox, 0, !show_highestmodseq ? 0 :
-			      STATUS_HIGHESTMODSEQ, &status) < 0)
+	storage = mailbox_get_storage(mailbox);
+	if ((ret = imap_expunge(mailbox, NULL)) < 0)
 		client_send_untagged_storage_error(client, storage);
 
-	if (mailbox_close(&mailbox) < 0)
-                client_send_untagged_storage_error(client, storage);
-	client_update_mailbox_flags(client, NULL);
-
-	if (!show_highestmodseq)
+	if ((client->enabled_features & MAILBOX_FEATURE_QRESYNC) != 0 &&
+	    ret > 0) {
+		/* we expunged something. since we're sending updated
+		   HIGHESTMODSEQ make sure the client sees all changes up to
+		   it by syncing the mailbox one last time. We wouldn't need
+		   to include our own expunge in there, but it's too much
+		   trouble to hide it. */
+		return cmd_sync_callback(cmd, 0, IMAP_SYNC_FLAG_SAFE,
+					 cmd_close_callback);
+	} else {
+		if (mailbox_sync(mailbox, 0, 0, NULL) < 0)
+			client_send_untagged_storage_error(client, storage);
+		cmd_close_finish(client);
 		client_send_tagline(cmd, "OK Close completed.");
-	else {
-		client_send_tagline(cmd, t_strdup_printf(
-			"OK [HIGHESTMODSEQ %llu] Close completed.",
-			(unsigned long long)status.highest_modseq));
+		return TRUE;
 	}
-	return TRUE;
 }
--- a/src/imap/cmd-expunge.c	Sun Mar 16 06:53:58 2008 +0200
+++ b/src/imap/cmd-expunge.c	Sun Mar 16 07:18:06 2008 +0200
@@ -51,14 +51,13 @@
 	if (search_arg == NULL)
 		return TRUE;
 
-	if (imap_expunge(client->mailbox, search_arg)) {
-		return cmd_sync_callback(cmd, 0, IMAP_SYNC_FLAG_SAFE,
-					 cmd_expunge_callback);
-	} else {
+	if (imap_expunge(client->mailbox, search_arg) < 0) {
 		client_send_storage_error(cmd,
 					  mailbox_get_storage(client->mailbox));
 		return TRUE;
 	}
+	return cmd_sync_callback(cmd, 0, IMAP_SYNC_FLAG_SAFE,
+				 cmd_expunge_callback);
 }
 
 bool cmd_expunge(struct client_command_context *cmd)
@@ -69,12 +68,11 @@
 		return TRUE;
 
 	cmd->client->sync_seen_deletes = FALSE;
-	if (imap_expunge(client->mailbox, NULL)) {
-		return cmd_sync_callback(cmd, 0, IMAP_SYNC_FLAG_SAFE,
-					 cmd_expunge_callback);
-	} else {
+	if (imap_expunge(client->mailbox, NULL) < 0) {
 		client_send_storage_error(cmd,
 					  mailbox_get_storage(client->mailbox));
 		return TRUE;
 	}
+	return cmd_sync_callback(cmd, 0, IMAP_SYNC_FLAG_SAFE,
+				 cmd_expunge_callback);
 }
--- a/src/imap/cmd-select.c	Sun Mar 16 06:53:58 2008 +0200
+++ b/src/imap/cmd-select.c	Sun Mar 16 07:18:06 2008 +0200
@@ -194,7 +194,7 @@
 				    "OK [READ-ONLY] Select completed." :
 				    "OK [READ-WRITE] Select completed.");
 	}
-	ctx->cmd->client->selecting = FALSE;
+	ctx->cmd->client->changing_mailbox = FALSE;
 	select_context_free(ctx);
 }
 
@@ -352,11 +352,8 @@
 		}
 	}
 
-	if (client->selecting) {
-		client_send_tagline(cmd, "Mailbox is already being selected");
-		return TRUE;
-	}
-	client->selecting = TRUE;
+	i_assert(!client->changing_mailbox);
+	client->changing_mailbox = TRUE;
 
 	if (client->mailbox != NULL) {
 		box = client->mailbox;
--- a/src/imap/cmd-unselect.c	Sun Mar 16 06:53:58 2008 +0200
+++ b/src/imap/cmd-unselect.c	Sun Mar 16 07:18:06 2008 +0200
@@ -12,6 +12,7 @@
 	if (!client_verify_open_mailbox(cmd))
 		return TRUE;
 
+	i_assert(!client->changing_mailbox);
 	client->mailbox = NULL;
 
 	storage = mailbox_get_storage(mailbox);
--- a/src/imap/imap-expunge.c	Sun Mar 16 06:53:58 2008 +0200
+++ b/src/imap/imap-expunge.c	Sun Mar 16 07:18:06 2008 +0200
@@ -5,17 +5,17 @@
 #include "mail-search.h"
 #include "imap-expunge.h"
 
-bool imap_expunge(struct mailbox *box, struct mail_search_arg *next_search_arg)
+int imap_expunge(struct mailbox *box, struct mail_search_arg *next_search_arg)
 {
 	struct mail_search_context *ctx;
         struct mailbox_transaction_context *t;
 	struct mail *mail;
 	struct mail_search_arg search_arg;
-	bool failed;
+	bool expunges = FALSE;
 
 	if (mailbox_is_readonly(box)) {
 		/* silently ignore */
-		return TRUE;
+		return 0;
 	}
 
 	memset(&search_arg, 0, sizeof(search_arg));
@@ -27,16 +27,19 @@
 	ctx = mailbox_search_init(t, NULL, &search_arg, NULL);
 
 	mail = mail_alloc(t, 0, NULL);
-	while (mailbox_search_next(ctx, mail) > 0)
+	while (mailbox_search_next(ctx, mail) > 0) {
 		mail_expunge(mail);
+		expunges = TRUE;
+	}
 	mail_free(&mail);
 
 	if (mailbox_search_deinit(&ctx) < 0) {
-		failed = TRUE;
 		mailbox_transaction_rollback(&t);
+		return -1;
 	} else {
-		failed = mailbox_transaction_commit(&t) < 0;
+		if (mailbox_transaction_commit(&t) < 0)
+			return -1;
 	}
 
-	return !failed;
+	return expunges ? 1 : 0;
 }
--- a/src/imap/imap-expunge.h	Sun Mar 16 06:53:58 2008 +0200
+++ b/src/imap/imap-expunge.h	Sun Mar 16 07:18:06 2008 +0200
@@ -1,6 +1,6 @@
 #ifndef IMAP_EXPUNGE_H
 #define IMAP_EXPUNGE_H
 
-bool imap_expunge(struct mailbox *box, struct mail_search_arg *next_search_arg);
+int imap_expunge(struct mailbox *box, struct mail_search_arg *next_search_arg);
 
 #endif