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