Merge pull request #58 from slusarz/54-imap-segfault-with-fts-flatcurve

54 imap segfault with fts flatcurve

.github/common/configs/dovecot.conf.issue-54

@@ -0,0 +1,3 @@
+!include /dovecot/configs/dovecot.conf
+
+mail_location = sdbox:~/sdbox:VOLATILEDIR=~/volatile

.github/common/fts-flatcurve-test.sh

@@ -113,6 +113,10 @@ run_test "Testing GitHub Issue #44 (Xapian indexed string too long)" \
 	/dovecot/configs/dovecot.conf \
 	/dovecot/imaptest/issue-44/issue-44
 
+run_test "Testing GitHub Issue #54 (VOLATILEDIR locking)" \
+	/dovecot/configs/dovecot.conf.issue-54 \
+	/dovecot/imaptest/small_mailbox
+
 TESTBOX=rotatetest
 run_test "Testing DB Rotation/Deletion (populating mailbox)" \
 	/dovecot/configs/dovecot.conf.issue-11 \

docs/configuration.md

@@ -40,6 +40,12 @@ plugin {
 }
 ```
 
+### NFS Recommendation
+
+To prevent concurrent writes to the Xapian database files, Dovecot relies on file locking.
+
+If using NFS, the [`VOLATILEDIR`](https://doc.dovecot.org/configuration_manual/nfs/#optimizations) parameter for the [`mail_location`](https://doc.dovecot.org/configuration_manual/mail_location/) configuration option should be used to perform this locking locally as opposed to on the remote server.
+
 ## Plugin Settings
 
 **The default parameters should be fine for most people.**

src/fts-backend-flatcurve-xapian.cpp

@@ -18,8 +18,10 @@ extern "C" {
 #include "array.h"
 #include "file-create-locked.h"
 #include "hash.h"
+#include "hex-binary.h"
 #include "mail-storage-private.h"
 #include "mail-search.h"
+#include "md5.h"
 #include "sleep.h"
 #include "str.h"
 #include "time-util.h"
@@ -512,20 +514,32 @@ fts_flatcurve_xapian_db_add(struct flatcurve_fts_backend *backend,
 
 static int fts_flatcurve_xapian_lock(struct flatcurve_fts_backend *backend)
 {
+	struct file_create_settings set;
 	struct flatcurve_xapian *x = backend->xapian;
 
-	if (x->lock_path == NULL)
-		x->lock_path = p_strdup_printf(
-			x->pool, "%s" FLATCURVE_XAPIAN_LOCK_FNAME,
-			str_c(backend->db_path));
-
-	struct file_create_settings set;
 	i_zero(&set);
 	set.lock_timeout_secs = FLATCURVE_XAPIAN_LOCK_TIMEOUT_SECS;
 	set.lock_settings.close_on_free = TRUE;
 	set.lock_settings.unlink_on_free = TRUE;
 	set.lock_settings.lock_method = backend->parsed_lock_method;
 
+	if (x->lock_path == NULL) {
+		if (str_len(backend->volatile_dir) > 0) {
+			unsigned char db_path_hash[MD5_RESULTLEN];
+			md5_get_digest(str_c(backend->db_path), str_len(backend->db_path),
+				db_path_hash);
+			x->lock_path = p_strdup_printf(
+				x->pool, "%s/" FLATCURVE_XAPIAN_LOCK_FNAME ".%s",
+				str_c(backend->volatile_dir),
+				binary_to_hex(db_path_hash, sizeof(db_path_hash)));
+			set.mkdir_mode = 0700;
+		} else {
+			x->lock_path = p_strdup_printf(
+				x->pool, "%s" FLATCURVE_XAPIAN_LOCK_FNAME,
+				str_c(backend->db_path));
+		}
+	}
+
 	bool created;
 	const char *error;
 	int ret = file_create_locked(x->lock_path, &set, &x->lock, &created, &error);
@@ -1420,8 +1434,8 @@ fts_flatcurve_xapian_optimize_box_do(struct flatcurve_fts_backend *backend,
 	if ((iter = fts_flatcurve_xapian_db_iter_init(backend, opts)) == NULL)
 		return FALSE;
 	while (fts_flatcurve_xapian_db_iter_next(iter)) {
-		if ((iter->type != FLATCURVE_XAPIAN_DB_TYPE_OPTIMIZE) &&
-		    (iter->type != FLATCURVE_XAPIAN_DB_TYPE_LOCK))
+		if ((iter->type == FLATCURVE_XAPIAN_DB_TYPE_INDEX) ||
+		    (iter->type == FLATCURVE_XAPIAN_DB_TYPE_CURRENT))
 			fts_flatcurve_xapian_delete(backend, iter->path);
 	}
 	fts_flatcurve_xapian_db_iter_deinit(&iter);

src/fts-backend-flatcurve.c

@@ -53,6 +53,7 @@ fts_backend_flatcurve_init(struct fts_backend *_backend, const char **error_r)
 	backend->boxname = str_new(backend->pool, 128);
 	backend->db_path = str_new(backend->pool, 256);
 	backend->fuser = fuser;
+	backend->volatile_dir = str_new(backend->pool, 128);
 
 	fuser->backend = backend;
 
@@ -74,6 +75,7 @@ fts_backend_flatcurve_close_mailbox(struct flatcurve_fts_backend *backend)
 
 		str_truncate(backend->boxname, 0);
 		str_truncate(backend->db_path, 0);
+		str_truncate(backend->volatile_dir, 0);
 	}
 
 	event_set_append_log_prefix(backend->event, FTS_FLATCURVE_DEBUG_PREFIX);
@@ -106,8 +108,9 @@ static void fts_backend_flatcurve_deinit(struct fts_backend *_backend)
 void fts_backend_flatcurve_set_mailbox(struct flatcurve_fts_backend *backend,
 				       struct mailbox *box)
 {
-	const char *path;
+	const char *path, *volatile_dir;
 	struct mail_storage *storage;
+	struct mail_user *user;
 
 	if (str_len(backend->boxname) &&
 	    (strcasecmp(box->vname, str_c(backend->boxname)) == 0))
@@ -128,6 +131,11 @@ void fts_backend_flatcurve_set_mailbox(struct flatcurve_fts_backend *backend,
 	storage = mailbox_get_storage(box);
 	backend->parsed_lock_method = storage->set->parsed_lock_method;
 
+	user = mail_storage_get_user(storage);
+	volatile_dir = mail_user_get_volatile_dir(user);
+	if (volatile_dir != NULL)
+		str_append(backend->volatile_dir, volatile_dir);
+
 	if (!backend->debug_init) {
 		e_debug(backend->event, "Xapian library version: %s",
 			fts_flatcurve_xapian_library_version());

src/fts-backend-flatcurve.h

@@ -12,7 +12,7 @@
 
 struct flatcurve_fts_backend {
 	struct fts_backend backend;
-	string_t *boxname, *db_path;
+	string_t *boxname, *db_path, *volatile_dir;
 
 	struct event *event;