Re: [Dovecot] Dovecot LDA munging INBOX access times?

[Timo Sirainen]

On Wed, 2007-06-13 at 17:52 -0500, Steven F Siirila wrote:
> We are running Dovecot 1.0.0 using mbox format (currently in the midst
> of conversion from UW IMAP).  We discovered today that the Dovecot LDA
> is accessing the user's INBOX at delivery time!

Well, there are two things:

1) It always makes sure that the mbox file ends with "\n". I guess this
isn't all that important, but I'm not really interested in just removing
the code.

2) If index files are fully synced, Dovecot writes X-UID: header. It
also updates nextuid field in X-IMAP: / X-IMAPbase: header of the first
message, which causes Dovecot to read() the file. The nextuid update
isn't really required, but I think some other bug shows up if it isn't
done.

It would be possible to check the atime before any reading is done and
then later after message is saved it could be updated back. Hmm. Well,
attached a patch that seems to work. I'm not sure if I want to add it to
v1.0.1.


diff -r 3099b59226af src/lib-storage/index/mbox/mbox-save.c
--- a/src/lib-storage/index/mbox/mbox-save.c	Thu Jun 14 05:31:03 2007 +0300
+++ b/src/lib-storage/index/mbox/mbox-save.c	Thu Jun 14 06:09:42 2007 +0300
@@ -27,6 +27,7 @@
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <netdb.h>
+#include <utime.h>
 
 #define MBOX_DELIVERY_ID_RAND_BYTES (64/8)
 
@@ -37,6 +38,7 @@ struct mbox_save_context {
 	struct mail_index_transaction *trans;
 	struct mail *mail;
 	uoff_t append_offset, mail_offset;
+	time_t orig_atime;
 
 	string_t *headers;
 	size_t space_end_idx;
@@ -84,6 +86,8 @@ static int mbox_seek_to_end(struct mbox_
 	fd = ctx->mbox->mbox_fd;
 	if (fstat(fd, &st) < 0)
                 return mbox_set_syscall_error(ctx->mbox, "fstat()");
+
+	ctx->orig_atime = st.st_atime;
 
 	*offset = (uoff_t)st.st_size;
 	if (st.st_size == 0)
@@ -653,15 +657,17 @@ int mbox_transaction_save_commit(struct 
 
 	i_assert(ctx->finished);
 
+	if (fstat(ctx->mbox->mbox_fd, &st) < 0) {
+		mbox_set_syscall_error(ctx->mbox, "fstat()");
+		ret = -1;
+	}
+
 	if (ctx->synced) {
 		mail_index_update_header(ctx->trans,
 			offsetof(struct mail_index_header, next_uid),
 			&ctx->next_uid, sizeof(ctx->next_uid), FALSE);
 
-		if (fstat(ctx->mbox->mbox_fd, &st) < 0) {
-			mbox_set_syscall_error(ctx->mbox, "fstat()");
-			ret = -1;
-		} else if (!ctx->mbox->mbox_sync_dirty) {
+		if (!ctx->mbox->mbox_sync_dirty && ret == 0) {
 			uint32_t sync_stamp = st.st_mtime;
 			uint64_t sync_size = st.st_size;
 
@@ -674,6 +680,16 @@ int mbox_transaction_save_commit(struct 
 		}
 	}
 
+	if (ret == 0 && ctx->orig_atime != st.st_atime) {
+		/* try to set atime back to its original value */
+		struct utimbuf buf;
+
+		buf.modtime = st.st_mtime;
+		buf.actime = ctx->orig_atime;
+		if (utime(ctx->mbox->path, &buf) < 0)
+			mbox_set_syscall_error(ctx->mbox, "utime()");
+	}
+
 	if (!ctx->synced && ctx->mbox->mbox_fd != -1 &&
 	    !ctx->mbox->mbox_writeonly && !ctx->mbox->ibox.fsync_disable) {
 		if (fdatasync(ctx->mbox->mbox_fd) < 0) {
diff -r 3099b59226af src/lib-storage/index/mbox/mbox-sync-private.h
--- a/src/lib-storage/index/mbox/mbox-sync-private.h	Thu Jun 14 05:31:03 2007 +0300
+++ b/src/lib-storage/index/mbox/mbox-sync-private.h	Thu Jun 14 06:08:04 2007 +0300
@@ -103,7 +103,7 @@ struct mbox_sync_context {
 	struct istream *input, *file_input;
 	int write_fd;
 
-	time_t orig_mtime;
+	time_t orig_mtime, orig_atime;
 	uoff_t orig_size;
 	struct stat last_stat;
 
diff -r 3099b59226af src/lib-storage/index/mbox/mbox-sync.c
--- a/src/lib-storage/index/mbox/mbox-sync.c	Thu Jun 14 05:31:03 2007 +0300
+++ b/src/lib-storage/index/mbox/mbox-sync.c	Thu Jun 14 06:19:49 2007 +0300
@@ -1589,6 +1589,7 @@ static int mbox_sync_do(struct mbox_sync
 	}
 	sync_ctx->last_stat = *st;
 	sync_ctx->orig_size = st->st_size;
+	sync_ctx->orig_atime = st->st_atime;
 	sync_ctx->orig_mtime = st->st_mtime;
 
 	if ((flags & MBOX_SYNC_FORCE_SYNC) != 0) {
@@ -1899,6 +1900,21 @@ __again:
                 ret = mbox_rewrite_base_uid_last(&sync_ctx);
 	}
 
+	if (ret == 0 && mbox->mbox_fd != -1 && mbox->ibox.keep_recent) {
+		/* try to set atime back to its original value */
+		struct utimbuf buf;
+		struct stat st;
+
+		if (fstat(mbox->mbox_fd, &st) < 0)
+			mbox_set_syscall_error(mbox, "fstat()");
+		else {
+			buf.modtime = st.st_mtime;
+			buf.actime = sync_ctx.orig_atime;
+			if (utime(mbox->path, &buf) < 0)
+				mbox_set_syscall_error(mbox, "utime()");
+		}
+	}
+
 	i_assert(lock_id != 0);
 
 	if (mbox->mbox_lock_type != F_RDLCK) {

signature.asc
Description: This is a digitally signed message part