<<< Date Index >>>     <<< Thread Index >>>

Re: [Mutt] #3410: Mutt crashes when two instances open the same mailbox



On Fri, Aug 06, 2010 at 09:37:40AM +0200, Matthias Andree wrote:
> Am 03.08.2010, 14:51 Uhr, schrieb Mutt:
> 
> >#3410: Mutt crashes when two instances open the same mailbox
> >--------------------+-------------------------------------------------------
> > Reporter:  vext01  |       Owner:  me
> >     Type:  defect  |      Status:  assigned
> > Priority:  major   |   Milestone:
> >Component:  mutt    |     Version:
> > Keywords:          |
> >--------------------+-------------------------------------------------------
> >
> >Comment(by vext01):
> >
> > I have an alias for configuring mutt, which looks like this:
> > configure_mutt='CPPFLAGS=-I/usr/local/include LDFLAGS=-L/usr/local/lib
> > ./configure --prefix=/opt/mutt --with-sasl --enable-smtp --enable-imap
> > --enable-hcache --with-ssl'

> (Only skimming through this, so I may have missed an attachment:)

Me too - but I just saw "--enable-hcache" above and then you wrote

> I'm not sure if it's practical to look at all free() calls though.

I had to get rid of a load of FREE(&h) calls which try to free memory
allocated and accounted by the db storing the hcache. AFAICT mutt should
not be freeing it - it is up to the db library.

I kept meaning to clean up a patch, but as I have ENOTIME, here is what
I am currently using. (It also enables boring but working Berkeley db 3.)
(Part of the clean up was to go through each different db backend and
check that mutt shouldn't free() - it was true for all those I did check.)

Hope this helps,

Patrick
diff -r 15b9d6f3284f configure.ac
--- a/configure.ac      Wed Apr 14 15:47:16 2010 -0700
+++ b/configure.ac      Fri Aug 06 11:54:35 2010 +0100
@@ -840,6 +840,8 @@
   [Don't use gdbm even if it is available]))
 AC_ARG_WITH(bdb, AC_HELP_STRING([--with-bdb@<:@=DIR@:>@],
   [Use BerkeleyDB4 if gdbm is not available]))
+AC_ARG_WITH(ndb, AC_HELP_STRING([--with-ndb],
+  [Use ndb (BerkelyDB1) if it is available]))
 
 db_found=no
 if test x$enable_hcache = xyes
@@ -882,6 +884,15 @@
         AC_MSG_ERROR([more than one header cache engine requested.])
       else
         db_requested=bdb
+      fi
+    fi
+    if test -n "$with_ndb" && test "$with_ndb" != "no"
+    then
+      if test "$db_requested" != "auto"
+      then
+        AC_MSG_ERROR([more than one header cache engine requested.])
+      else
+        db_requested=ndb
       fi
     fi
     
@@ -967,6 +978,28 @@
         then
           AC_MSG_ERROR([GDBM could not be used. Check config.log for details.])
         fi
+    fi
+
+    dnl -- NDB -- XXX while experimental insist on explicit request for ndb
+    if test "$db_requested" = ndb
+    then
+        AC_MSG_CHECKING([for BSD licenced Berkley DB])
+        AC_LINK_IFELSE(
+            [AC_LANG_PROGRAM([
+                #include <sys/types.h>
+                #include <limits.h>
+                #include <db.h>
+                #include <fcntl.h>
+            ],[
+                DB *db;
+                db = dbopen(NULL, 0, 0, 0, NULL);
+            ])],[
+            AC_MSG_RESULT(yes)
+            AC_DEFINE(HAVE_NDB, 1, [BSD licenced Berkeley DB Support])
+            db_found=ndb
+        ],[
+            AC_MSG_RESULT(no)
+        ])
     fi
 
     dnl -- BDB --
diff -r 15b9d6f3284f hcache.c
--- a/hcache.c  Wed Apr 14 15:47:16 2010 -0700
+++ b/hcache.c  Fri Aug 06 11:54:35 2010 +0100
@@ -30,8 +30,8 @@
 #include <tcbdb.h>
 #elif HAVE_GDBM
 #include <gdbm.h>
-#elif HAVE_DB4
-#include <db.h>
+#elif defined(HAVE_DB4) || defined(HAVE_NDB)
+#include "/usr/include/db.h"
 #endif
 
 #include <errno.h>
@@ -65,6 +65,13 @@
 static struct header_cache
 {
   GDBM_FILE db;
+  char *folder;
+  unsigned int crc;
+} HEADER_CACHE;
+#elif HAVE_NDB
+static struct header_cache
+{
+  DB *db;
   char *folder;
   unsigned int crc;
 } HEADER_CACHE;
@@ -717,6 +724,9 @@
 #elif HAVE_DB4
   DBT key;
   DBT data;
+#elif HAVE_NDB
+  DBT key, data;
+  void *ret = NULL;
 #endif
   
   if (!h)
@@ -754,6 +764,21 @@
   data = gdbm_fetch(h->db, key);
   
   return data.dptr;
+#elif HAVE_NDB
+  key.data = path;
+  key.size = ksize;
+  switch (h->db->get(h->db, &key, &data, 0)) {
+    case -1: dprint (1, (debugfile,
+                 "mutt_hcache_fetch_raw: ndb get key \"%s\" returned %s 
(%d)\n",
+                 path, strerror(errno), errno));
+             ret = NULL;
+             break;
+    case  1: ret = NULL; /* key not found */
+             break;
+    case  0: ret = data.data;
+             break;
+  }
+  return ret;
 #endif
 }
 
@@ -791,6 +816,9 @@
 #elif HAVE_DB4
   DBT key;
   DBT databuf;
+#elif HAVE_NDB
+  int ret;
+  DBT key, databuf;
 #endif
   
   if (!h)
@@ -827,6 +855,25 @@
   databuf.dptr = data;
   
   return gdbm_store(h->db, key, databuf, GDBM_REPLACE);
+#elif HAVE_NDB
+  key.data = path;
+  key.size = ksize;
+  databuf.data = data;
+  databuf.size = dlen;
+  ret = h->db->put(h->db, &key, &databuf, 0);
+  switch (ret) {
+    case -1: dprint (1, (debugfile,
+                 "mutt_hcache_store_raw: ndb get key \"%s\" returned %s 
(%d)\n",
+                 path, strerror(errno), errno));
+             break;
+    case  1: dprint (1, (debugfile,      /* this should never happen */
+                 "mutt_hcache_store_raw: key \"%s\" already exists and the 
R_NOOVERWRITE flag is set",
+                 path));
+             break;
+    case  0: /* success */
+             break;
+  }
+  return ret; /* even though callers ignore the return value */
 #endif
 }
 
@@ -992,6 +1039,66 @@
 
   return gdbm_delete(h->db, key);
 }
+#elif HAVE_NDB
+
+static int
+hcache_open_ndb (struct header_cache* h, const char* path)
+{
+  if (!h)
+    return -1;
+
+  h->db = dbopen(path, O_RDWR | O_CREAT | O_EXLOCK,
+                 S_IRUSR | S_IWUSR, DB_BTREE, NULL);
+  if (h->db == NULL)
+    dprint(1, (debugfile,
+        "hcache_open_ndb: Couldn't open database read-write: %s (%d)\n",
+        strerror(errno), errno));
+  else
+    return 0;
+
+  /* if rw failed try ro */
+  h->db = dbopen(path, O_RDONLY | O_CREAT | O_EXLOCK,
+                 S_IRUSR | S_IWUSR, DB_BTREE, NULL);
+  if (h->db == NULL)
+    dprint(1, (debugfile,
+        "hcache_open_ndb: Couldn't open database read-only: %s (%d)\n",
+        strerror(errno), errno));
+  else
+    return 0;
+
+  return -1;
+}
+
+void
+mutt_hcache_close(header_cache_t *h)
+{
+  if (!h)
+    return;
+
+  /* needed? flock(h->db->fd(db), LOCK_UN); */
+  h->db->close(h->db);
+  FREE(&h->folder);
+  FREE(&h);
+}
+
+int
+mutt_hcache_delete(header_cache_t *h, const char *filename,
+                  size_t(*keylen) (const char *fn))
+{
+  DBT key;
+  char path[_POSIX_PATH_MAX];
+
+  if (!h)
+    return -1;
+
+  strncpy(path, h->folder, sizeof (path));
+  safe_strcat(path, sizeof (path), filename);
+
+  key.data = path;
+  key.size = strlen(h->folder) + keylen(path + strlen(h->folder));
+
+  return h->db->del(h->db, &key, R_CURSOR);
+}
 #elif HAVE_DB4
 
 static void
@@ -1112,11 +1219,13 @@
 #if HAVE_QDBM
   hcache_open = hcache_open_qdbm;
 #elif HAVE_TC
-  hcache_open= hcache_open_tc;
+  hcache_open = hcache_open_tc;
 #elif HAVE_GDBM
   hcache_open = hcache_open_gdbm;
 #elif HAVE_DB4
   hcache_open = hcache_open_db4;
+#elif HAVE_NDB
+  hcache_open = hcache_open_ndb;
 #endif
 
   h->db = NULL;
@@ -1169,4 +1278,9 @@
 {
   return "tokyocabinet " _TC_VERSION;
 }
+#elif HAVE_NDB
+const char *mutt_hcache_backend (void)
+{
+    return "Berkeley DB 1.xx using btree";
+}
 #endif
diff -r 15b9d6f3284f imap/imap.c
--- a/imap/imap.c       Wed Apr 14 15:47:16 2010 -0700
+++ b/imap/imap.c       Fri Aug 06 11:54:35 2010 +0100
@@ -1635,8 +1635,8 @@
     {
       if (!status)
       {
-        FREE (&uidvalidity);
-        FREE (&uidnext);
+        // XXX PRLW FREE (&uidvalidity);
+        // XXX PRLW FREE (&uidnext);
         return imap_mboxcache_get (idata, mbox, 1);
       }
       status->uidvalidity = *uidvalidity;
@@ -1644,8 +1644,8 @@
       dprint (3, (debugfile, "mboxcache: hcache uidvalidity %d, uidnext %d\n",
                   status->uidvalidity, status->uidnext));
     }
-    FREE (&uidvalidity);
-    FREE (&uidnext);
+    // XXX PRLW FREE (&uidvalidity);
+    // XXX PRLW FREE (&uidnext);
   }
 #endif
 
diff -r 15b9d6f3284f imap/message.c
--- a/imap/message.c    Wed Apr 14 15:47:16 2010 -0700
+++ b/imap/message.c    Fri Aug 06 11:54:35 2010 +0100
@@ -129,11 +129,11 @@
     if (puidnext)
     {
       uidnext = *puidnext;
-      FREE (&puidnext);
+      // XXX PRLW FREE (&puidnext);
     }
     if (uid_validity && uidnext && *uid_validity == idata->uid_validity)
       evalhc = 1;
-    FREE (&uid_validity);
+    // XXX PRLW FREE (&uid_validity);
   }
   if (evalhc)
   {
diff -r 15b9d6f3284f imap/util.c
--- a/imap/util.c       Wed Apr 14 15:47:16 2010 -0700
+++ b/imap/util.c       Fri Aug 06 11:54:35 2010 +0100
@@ -131,7 +131,7 @@
       h = mutt_hcache_restore ((unsigned char*)uv, NULL);
     else
       dprint (3, (debugfile, "hcache uidvalidity mismatch: %u", *uv));
-    FREE (&uv);
+    // XXX PRLW FREE (&uv);
   }
 
   return h;
diff -r 15b9d6f3284f mutt_socket.c
--- a/mutt_socket.c     Wed Apr 14 15:47:16 2010 -0700
+++ b/mutt_socket.c     Fri Aug 06 11:54:35 2010 +0100
@@ -114,7 +114,7 @@
   int rc;
   int sent = 0;
 
-  dprint (dbg, (debugfile,"%d> %s", conn->fd, buf));
+  dprint (dbg, (debugfile, "%d> %s", conn->fd, buf));
 
   if (conn->fd < 0)
   {