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)
{