Changeset 5494:53d9210aa4ee

Timestamp:

2008-08-25 00:16:30 (3 months ago)

Author:

Brendan Cully <brendan@…>

Branch:

HEAD

Message:

Rework IMAP command queueing to allow pipelining to be disabled.
IDLE handling has been better abstracted, and there are fewer entry
points to the IMAP command issuing machinery. Any commands that
are simply queued may be executed whenever the pipeline fills,
instead of requiring explicit handling in the caller.

Tested on my Cyrus server, but I wouldn't be surprise if this causes
new problems.

Location:

imap

Files:

• command.c (modified) (8 diffs)
• imap.c (modified) (11 diffs)
• imap_private.h (modified) (4 diffs)
• message.c (modified) (2 diffs)

imap/command.c

@@ -2 +2 @@
  * Copyright (C) 1996-8 Michael R. Elkins <me@mutt.org>
  * Copyright (C) 1996-9 Brandon Long <blong@fiction.net>
- * Copyright (C) 1999-2006 Brendan Cully <brendan@kublai.com>
+ * Copyright (C) 1999-2008 Brendan Cully <brendan@kublai.com>
  * 
  *     This program is free software; you can redistribute it and/or modify
@@ -38 +38 @@
 
 /* forward declarations */
+static int cmd_start (IMAP_DATA* idata, const char* cmdstr, int flags);
+static int cmd_queue_full (IMAP_DATA* idata);
+static int cmd_queue (IMAP_DATA* idata, const char* cmdstr);
 static IMAP_COMMAND* cmd_new (IMAP_DATA* idata);
 static int cmd_status (const char *s);
@@ -68 +71 @@
 };
 
-/* imap_cmd_queue: Add command to command queue. Fails if the queue is full. */
-int imap_cmd_queue (IMAP_DATA* idata, const char* cmdstr)
-{
-  IMAP_COMMAND* cmd;
-
-  if (idata->status == IMAP_FATAL)
-  {
-    cmd_handle_fatal (idata);
-    return IMAP_CMD_BAD;
-  }
-
-  if (!(cmd = cmd_new (idata)))
-    return IMAP_CMD_BAD;
-
-  if (mutt_buffer_printf (idata->cmdbuf, "%s%s %s\r\n",
-      idata->state == IMAP_IDLE ? "DONE\r\n" : "", cmd->seq, cmdstr) < 0)
-    return IMAP_CMD_BAD;
-
-  if (idata->state == IMAP_IDLE)
-    idata->state = IMAP_SELECTED;
-
-  return 0;
-}
-
 /* imap_cmd_start: Given an IMAP command, send it to the server.
  *   If cmdstr is NULL, sends queued commands. */
 int imap_cmd_start (IMAP_DATA* idata, const char* cmdstr)
 {
-  int rc;
-
-  if (cmdstr && (rc = imap_cmd_queue (idata, cmdstr)) < 0)
-    return rc;
-
-  /* don't write old or empty commands */
-  if (idata->cmdbuf->dptr == idata->cmdbuf->data)
-    return IMAP_CMD_BAD;
-
-  rc = mutt_socket_write (idata->conn, idata->cmdbuf->data);
-  idata->cmdbuf->dptr = idata->cmdbuf->data;
-
-  return (rc < 0) ? IMAP_CMD_BAD : 0;
+  return cmd_start (idata, cmdstr, 0);
 }
 
@@ -236 +203 @@
   int rc;
 
-  if (idata->status == IMAP_FATAL)
+  if ((rc = cmd_start (idata, cmdstr, flags)) < 0)
+    return rc;
+
+  if (rc < 0)
   {
     cmd_handle_fatal (idata);
@@ -242 +212 @@
   }
 
-  if (cmdstr && (rc = imap_cmd_queue (idata, cmdstr)) < 0)
-    return rc;
-  
-  /* don't write old or empty commands */
-  if (idata->cmdbuf->dptr == idata->cmdbuf->data)
-    return IMAP_CMD_BAD;
-  
-  rc = mutt_socket_write_d (idata->conn, idata->cmdbuf->data, -1,
-    flags & IMAP_CMD_PASS ? IMAP_LOG_PASS : IMAP_LOG_CMD);
-  idata->cmdbuf->dptr = idata->cmdbuf->data;
-
-  if (rc < 0)
-  {
-    cmd_handle_fatal (idata);
-    return -1;
-  }
+  if (flags & IMAP_CMD_QUEUE)
+    return 0;
 
   do
@@ -324 +280 @@
 }
 
+/* imap_cmd_idle: Enter the IDLE state. */
+int imap_cmd_idle (IMAP_DATA* idata)
+{
+  int rc;
+
+  imap_cmd_start (idata, "IDLE");
+  do
+    rc = imap_cmd_step (idata);
+  while (rc == IMAP_CMD_CONTINUE);
+
+  if (rc == IMAP_CMD_RESPOND)
+  {
+    /* successfully entered IDLE state */
+    idata->state = IMAP_IDLE;
+    /* queue automatic exit when next command is issued */
+    mutt_buffer_printf (idata->cmdbuf, "DONE\r\n");
+    rc = IMAP_CMD_OK;
+  }
+  if (rc != IMAP_CMD_OK)
+  {
+    dprint (1, (debugfile, "imap_cmd_idle: error starting IDLE\n"));
+    return -1;
+  }
+  
+  return 0;
+}
+
+static int cmd_queue_full (IMAP_DATA* idata)
+{
+  if ((idata->nextcmd + 1) % IMAP_PIPELINE_DEPTH == idata->lastcmd)
+    return 1;
+
+  return 0;
+}
+
 /* sets up a new command control block and adds it to the queue.
  * Returns NULL if the pipeline is full. */
@@ -330 +321 @@
   IMAP_COMMAND* cmd;
 
-  if ((idata->nextcmd + 1) % IMAP_PIPELINE_DEPTH == idata->lastcmd)
-  {
-    dprint (2, (debugfile, "cmd_new: IMAP command queue full\n"));
+  if (cmd_queue_full (idata))
+  {
+    dprint (3, (debugfile, "cmd_new: IMAP command queue full\n"));
     return NULL;
   }
@@ -346 +337 @@
 
   return cmd;
+}
+
+/* queues command. If the queue is full, attempts to drain it. */
+static int cmd_queue (IMAP_DATA* idata, const char* cmdstr)
+{
+  IMAP_COMMAND* cmd;
+  int rc;
+
+  if (cmd_queue_full (idata))
+  {
+    dprint (3, (debugfile, "Draining IMAP command pipeline\n"));
+
+    rc = imap_exec (idata, NULL, IMAP_CMD_FAIL_OK);
+
+    if (rc < 0 && rc != -2)
+      return rc;
+  }
+
+  if (!(cmd = cmd_new (idata)))
+    return IMAP_CMD_BAD;
+
+  if (mutt_buffer_printf (idata->cmdbuf, "%s %s\r\n", cmd->seq, cmdstr) < 0)
+    return IMAP_CMD_BAD;
+
+  return 0;
+}
+
+static int cmd_start (IMAP_DATA* idata, const char* cmdstr, int flags)
+{
+  int rc;
+
+  if (idata->status == IMAP_FATAL)
+  {
+    cmd_handle_fatal (idata);
+    return -1;
+  }
+
+  if (cmdstr && ((rc = cmd_queue (idata, cmdstr)) < 0))
+    return rc;
+
+  if (flags & IMAP_CMD_QUEUE)
+    return 0;
+
+  if (idata->cmdbuf->dptr == idata->cmdbuf->data)
+    return IMAP_CMD_BAD;
+
+  rc = mutt_socket_write_d (idata->conn, idata->cmdbuf->data, -1,
+                            flags & IMAP_CMD_PASS ? IMAP_LOG_PASS : IMAP_LOG_CMD);
+  idata->cmdbuf->dptr = idata->cmdbuf->data;
+
+  /* unidle when command queue is flushed */
+  if (idata->state == IMAP_IDLE)
+    idata->state = IMAP_SELECTED;
+
+  return (rc < 0) ? IMAP_CMD_BAD : 0;
 }
 

imap/imap.c

@@ -2 +2 @@
  * Copyright (C) 1996-8 Michael R. Elkins <me@mutt.org>
  * Copyright (C) 1996-9 Brandon Long <blong@fiction.net>
- * Copyright (C) 1999-2007 Brendan Cully <brendan@kublai.com>
+ * Copyright (C) 1999-2008 Brendan Cully <brendan@kublai.com>
  * 
  *     This program is free software; you can redistribute it and/or modify
@@ -348 +348 @@
   }
   if (!conn)
-          return NULL; /* this happens when the initial connection fails */
+    return NULL; /* this happens when the initial connection fails */
 
   if (!idata)
@@ -384 +384 @@
   {
     /* capabilities may have changed */
-    imap_cmd_queue (idata, "CAPABILITY");
+    imap_exec (idata, "CAPABILITY", IMAP_CMD_QUEUE);
     /* get root delimiter, '/' as default */
     idata->delim = '/';
-    imap_cmd_queue (idata, "LIST \"\" \"\"");
+    imap_exec (idata, "LIST \"\" \"\"", IMAP_CMD_QUEUE);
     if (option (OPTIMAPCHECKSUBSCRIBED))
-      imap_cmd_queue (idata, "LSUB \"\" \"*\"");
+      imap_exec (idata, "LSUB \"\" \"*\"", IMAP_CMD_QUEUE);
     /* we may need the root delimiter before we open a mailbox */
     imap_exec (idata, NULL, IMAP_CMD_FAIL_OK);
@@ -599 +599 @@
   {
     snprintf (bufout, sizeof (bufout), "MYRIGHTS %s", buf);
-    imap_cmd_queue (idata, bufout);
+    imap_exec (idata, bufout, IMAP_CMD_QUEUE);
   }
   /* assume we have all rights if ACL is unavailable */
@@ -1073 +1073 @@
     rc++;
     mutt_buffer_printf (buf, " +FLAGS.SILENT (%s)", name);
-    imap_cmd_queue (idata, buf->data);
+    imap_exec (idata, buf->data, IMAP_CMD_QUEUE);
   }
   buf->dptr = buf->data;
@@ -1081 +1081 @@
     rc++;
     mutt_buffer_printf (buf, " -FLAGS.SILENT (%s)", name);
-    imap_cmd_queue (idata, buf->data);
+    imap_exec (idata, buf->data, IMAP_CMD_QUEUE);
   }
   
@@ -1259 +1259 @@
   if (expunge && ctx->closing)
   {
-    imap_cmd_queue (idata, "CLOSE");
+    imap_exec (idata, "CLOSE", IMAP_CMD_QUEUE);
     idata->state = IMAP_AUTHENTICATED;
   }
@@ -1297 +1297 @@
        * and the mailbox is unchanged, so we may have to close here */
       if (!ctx->deleted)
-        imap_cmd_queue (idata, "CLOSE");
+        imap_exec (idata, "CLOSE", IMAP_CMD_QUEUE);
       if (idata->state == IMAP_IDLE)
       {
@@ -1353 +1353 @@
       && (idata->state != IMAP_IDLE || time(NULL) >= idata->lastread + ImapKeepalive))
   {
-    imap_cmd_start (idata, "IDLE");
-    idata->state = IMAP_IDLE;
-    do
-      result = imap_cmd_step (idata);
-    while (result == IMAP_CMD_CONTINUE);
-    /* it's possible that we were notified and fetched mail before
-     * getting to the +, in which case we've automatically unidled. */
-    if (result != IMAP_CMD_RESPOND && result != IMAP_CMD_OK)
-    {
-      dprint (1, (debugfile, "Error starting IDLE\n"));
-      idata->state = IMAP_SELECTED;
+    if (imap_cmd_idle (idata) < 0)
       return -1;
-    }
   }
   if (idata->state == IMAP_IDLE)
@@ -1487 +1476 @@
               "STATUS %s (UIDNEXT UIDVALIDITY UNSEEN RECENT)", munged);
 
-    if (imap_cmd_queue (idata, command) < 0)
-    {
-      /* pipeline must be full, drain it */
-      dprint (2, (debugfile, "IMAP command pipeline full, draining\n"));
-
-      if (imap_exec (idata, NULL, IMAP_CMD_FAIL_OK) == -1)
-        dprint (1, (debugfile, "Error polling mailboxes\n"));
-      
-      if (imap_cmd_queue (idata, command) < 0) {
-        /* real trouble */
-        dprint (1, (debugfile, "Error queueing command\n"));
-        return 0;
-      }
+    if (imap_exec (idata, command, IMAP_CMD_QUEUE) < 0)
+    {
+      dprint (1, (debugfile, "Error queueing command\n"));
+      return 0;
     }
   }
@@ -1551 +1531 @@
   if (queue)
   {
-    imap_cmd_queue (idata, buf);
+    imap_exec (idata, buf, IMAP_CMD_QUEUE);
     queued = 1;
     return 0;

imap/imap_private.h

@@ -1 +1 @@
 /*
  * Copyright (C) 1996-9 Brandon Long <blong@fiction.net>
- * Copyright (C) 1999-2005 Brendan Cully <brendan@kublai.com>
+ * Copyright (C) 1999-2008 Brendan Cully <brendan@kublai.com>
  * 
  *     This program is free software; you can redistribute it and/or modify
@@ -70 +70 @@
 #define IMAP_CMD_FAIL_OK (1<<0)
 #define IMAP_CMD_PASS    (1<<1)
+#define IMAP_CMD_QUEUE   (1<<2)
 
 enum
@@ -244 +245 @@
 
 /* command.c */
-int imap_cmd_queue (IMAP_DATA* idata, const char* cmdstr);
 int imap_cmd_start (IMAP_DATA* idata, const char* cmd);
 int imap_cmd_step (IMAP_DATA* idata);
@@ -250 +250 @@
 int imap_code (const char* s);
 int imap_exec (IMAP_DATA* idata, const char* cmd, int flags);
+int imap_cmd_idle (IMAP_DATA* idata);
 
 /* message.c */

imap/message.c

@@ -1 +1 @@
 /*
  * Copyright (C) 1996-9 Brandon Long <blong@fiction.net>
- * Copyright (C) 1999-2007 Brendan Cully <brendan@kublai.com>
+ * Copyright (C) 1999-2008 Brendan Cully <brendan@kublai.com>
  * 
  *     This program is free software; you can redistribute it and/or modify
@@ -144 +144 @@
     imap_cmd_start (idata, buf);
   
-    for (msgno = msgbegin; msgno <= msgend ; msgno++)
+    rc = IMAP_CMD_CONTINUE;
+    for (msgno = msgbegin; rc == IMAP_CMD_CONTINUE; msgno++)
     {
       mutt_progress_update (&progress, msgno + 1, -1);