lib-imap-client: Fix/clarify selection state handling

The old code assumed that selected_box would be non-NULL while a new mailbox
is being selected. That's not true though, the imapc-storage code closes the
old mailbox before selecting the next one. So the QRESYNC-specific code for
tracking selected-state was never being used. Fixed this, and clarified in
general how the selected-state is supposed to work.

diff --git a/src/lib-imap-client/imapc-client.c b/src/lib-imap-client/imapc-client.c
index 22ddc45094ff8b3f68ee4ae3184567ebdcae5ff3..fe9bc9ee3fc94f5625f8a58af1b2449dd4cdd6f0 100644 (file)
--- a/src/lib-imap-client/imapc-client.c
+++ b/src/lib-imap-client/imapc-client.c
@@ -417,7 +417,7 @@ void imapc_client_mailbox_close(struct imapc_client_mailbox **_box)
        box->closing = TRUE;
 
        /* cancel any pending commands */
-       imapc_connection_unselect(box);
+       imapc_connection_unselect(box, FALSE);
 
        if (box->reconnecting) {
                /* need to abort the reconnection so it won't try to access

diff --git a/src/lib-imap-client/imapc-connection.c b/src/lib-imap-client/imapc-connection.c
index 9072b491da7c07ebb8e0dd6834de7c603064e1e6..fe5bc632348cb092372693782b67014ca9f57d71 100644 (file)
--- a/src/lib-imap-client/imapc-connection.c
+++ b/src/lib-imap-client/imapc-connection.c
@@ -102,7 +102,20 @@ struct imapc_connection {
        struct timeval last_connect;
        unsigned int reconnect_count;
 
-       struct imapc_client_mailbox *qresync_selecting_box, *selected_box;
+       /* If QRESYNC isn't used, this is set immediately after issuing
+          SELECT/EXAMINE. We could differentiate better whether a mailbox is
+          "being selected" vs "fully selected", but that code is already in
+          the imapc-storage side so it would have to be moved or duplicated
+          here. And since nothing actually cares about this distinction (yet),
+          don't bother with it for now. This is set to NULL when the mailbox
+          is closed from imapc-storage point of view, even if the server is
+          still in selected state (see selected_on_server). */
+       struct imapc_client_mailbox *selected_box;
+       /* If QRESYNC is used, this is set when SELECT/EXAMINE is issued.
+          If the server is already in selected state, the selected_box is most
+          likely already NULL at this point, because imapc-storage has closed
+          it. */
+       struct imapc_client_mailbox *qresync_selecting_box;
        enum imapc_connection_state state;
        char *disconnect_reason;
 
@@ -141,6 +154,10 @@ struct imapc_connection {
        bool idle_stopping:1;
        bool idle_plus_waiting:1;
        bool select_waiting_reply:1;
+       /* TRUE if IMAP server is in SELECTED state. select_box may be NULL
+          though, if we already closed the mailbox from client point of
+          view. */
+       bool selected_on_server:1;
 };
 
 static void imapc_connection_capability_cb(const struct imapc_command_reply *reply,
@@ -401,6 +418,7 @@ static void imapc_connection_set_state(struct imapc_connection *conn,
                conn->select_waiting_reply = FALSE;
                conn->qresync_selecting_box = NULL;
                conn->selected_box = NULL;
+               conn->selected_on_server = FALSE;
                /* fall through */
        case IMAPC_CONNECTION_STATE_DONE:
                /* if we came from imapc_client_get_capabilities(), stop so
@@ -779,6 +797,8 @@ imapc_connection_handle_resp_text_code(struct imapc_connection *conn,
                if (conn->qresync_selecting_box != NULL) {
                        conn->selected_box = conn->qresync_selecting_box;
                        conn->qresync_selecting_box = NULL;
+               } else {
+                       conn->selected_on_server = FALSE;
                }
        }
        return 0;
@@ -1493,7 +1513,7 @@ static int imapc_connection_input_tagged(struct imapc_connection *conn)
            (cmd->flags & IMAPC_COMMAND_FLAG_SELECT) != 0 &&
            conn->selected_box != NULL) {
                /* EXAMINE/SELECT failed: mailbox is no longer selected */
-               imapc_connection_unselect(conn->selected_box);
+               imapc_connection_unselect(conn->selected_box, TRUE);
        }
 
        if (conn->reconnect_command_count > 0 &&
@@ -2093,7 +2113,7 @@ static void imapc_connection_set_selecting(struct imapc_client_mailbox *box)
 
        i_assert(conn->qresync_selecting_box == NULL);
 
-       if (conn->selected_box != NULL &&
+       if (conn->selected_on_server &&
            (conn->capabilities & IMAPC_CAPABILITY_QRESYNC) != 0) {
                /* server will send a [CLOSED] once selected mailbox is
                   closed */
@@ -2102,6 +2122,7 @@ static void imapc_connection_set_selecting(struct imapc_client_mailbox *box)
                /* we'll have to assume that all the future untagged messages
                   are for the mailbox we're selecting */
                conn->selected_box = box;
+               conn->selected_on_server = TRUE;
        }
        conn->select_waiting_reply = TRUE;
 }
@@ -2448,17 +2469,37 @@ imapc_connection_get_capabilities(struct imapc_connection *conn)
        return conn->capabilities;
 }
 
-void imapc_connection_unselect(struct imapc_client_mailbox *box)
+void imapc_connection_unselect(struct imapc_client_mailbox *box,
+                              bool via_tagged_reply)
 {
        struct imapc_connection *conn = box->conn;
 
-       if (conn->selected_box != NULL || conn->qresync_selecting_box != NULL) {
-               i_assert(conn->selected_box == box ||
-                        conn->qresync_selecting_box == box);
-
-               conn->selected_box = NULL;
+       if (conn->select_waiting_reply) {
+               /* Mailbox closing was requested before SELECT/EXAMINE
+                  replied. The connection state is now unknown and
+                  shouldn't be used anymore. */
+               imapc_connection_disconnect(conn);
+       } else if (conn->qresync_selecting_box == NULL &&
+                  conn->selected_box == NULL) {
+               /* There is no mailbox selected currently. */
+               i_assert(!via_tagged_reply);
+       } else {
+               /* Mailbox was closed in a known state. Either due to
+                  SELECT/EXAMINE failing (via_tagged_reply) or by
+                  imapc-storage after the mailbox was already fully
+                  selected. */
+               i_assert(conn->qresync_selecting_box == box ||
+                        conn->selected_box == box);
                conn->qresync_selecting_box = NULL;
+               conn->selected_box = NULL;
+               if (via_tagged_reply)
+                       conn->selected_on_server = FALSE;
+               else {
+                       /* We didn't actually send UNSELECT command, so don't
+                          touch selected_on_server state. */
+               }
        }
+
        imapc_connection_send_idle_done(conn);
        imapc_connection_abort_commands(conn, box, FALSE);
 }

diff --git a/src/lib-imap-client/imapc-connection.h b/src/lib-imap-client/imapc-connection.h
index 736eed045e9b5165f79b0cfd8ab452887666de8a..3f63cea025041727c6a4c02daa98f83cdf96f0c3 100644 (file)
--- a/src/lib-imap-client/imapc-connection.h
+++ b/src/lib-imap-client/imapc-connection.h
@@ -48,7 +48,8 @@ imapc_connection_cmd(struct imapc_connection *conn,
                     imapc_command_callback_t *callback, void *context)
        ATTR_NULL(3);
 
-void imapc_connection_unselect(struct imapc_client_mailbox *box);
+void imapc_connection_unselect(struct imapc_client_mailbox *box,
+                              bool via_tagged_reply);
 
 enum imapc_connection_state
 imapc_connection_get_state(struct imapc_connection *conn);