Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

54 imap segfault with fts flatcurve #58

Merged
merged 4 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/common/configs/dovecot.conf.issue-54
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
!include /dovecot/configs/dovecot.conf

mail_location = sdbox:~/sdbox:VOLATILEDIR=~/volatile
4 changes: 4 additions & 0 deletions .github/common/fts-flatcurve-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
6 changes: 6 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.**
Expand Down
30 changes: 22 additions & 8 deletions src/fts-backend-flatcurve-xapian.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 9 additions & 1 deletion src/fts-backend-flatcurve.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down Expand Up @@ -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))
Expand All @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion src/fts-backend-flatcurve.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down