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 adb4dcb044e49f0fcbeb6281e29c63ebea5b893e..5806a80c500915962afab5140162586bbcc84da2 100644 (file)
--- a/src/lib-imap-client/imapc-client.c
+++ b/src/lib-imap-client/imapc-client.c
@@ -416,7 +416,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 1c3d67de5e85bd322f6143750717c8bed24a7af4..f0254034817392aea872c993850db404cfcd55c3 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,
@@ -404,6 +421,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
@@ -785,6 +803,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;
@@ -1501,7 +1521,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 &&
@@ -2111,7 +2131,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 */
@@ -2120,6 +2140,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;
 }
@@ -2466,17 +2487,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 8fda69260c0c7923353d4da7ea63cbcadefe9c4d..20c249e6f8e7ef82f7312bec9f4d00750a8e682f 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);