From: Austin Clements <amdragon@MIT.EDU>
To: notmuch@notmuchmail.org
Subject: [PATCH 1/2] new: Don't update DB mtime if FS mtime equals wall-clock
	time.
Date: Wed, 29 Jun 2011 03:10:54 -0400
Message-Id: <1309331455-19584-1-git-send-email-amdragon@mit.edu>
Cc: amdragon@mit.edu
Precedence: list

This fixes a race where multiple message deliveries in the same second
with an intervening notmuch new could result in messages being ignored
by notmuch (at least, until a later delivery forced a rescan).
Because mtimes only have second granularity, later deliveries in the
same second won't change the directory mtime, and hence won't trigger
notmuch new to rescan the directory.  This situation can only occur
when notmuch new is being run at the same second as the directory's
modification time, so simply don't update the saved mtime in this
case.

This very race happens all over the test suite, and is currently
compensated for with increment_mtime (and, occasionally, luck).  With
this change, increment_mtime becomes unnecessary.
---
 notmuch-new.c |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 0fa2a3c..c180591 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -213,6 +213,7 @@ _entries_resemble_maildir (struct dirent **entries, int count)
  *     information is lost from the database).
  *
  *   o Tell the database to update its time of 'path' to 'fs_mtime'
+ *     if fs_mtime isn't the current wall-clock time.
  */
 static notmuch_status_t
 add_files_recursive (notmuch_database_t *notmuch,
@@ -230,6 +231,7 @@ add_files_recursive (notmuch_database_t *notmuch,
     notmuch_directory_t *directory;
     notmuch_filenames_t *db_files = NULL;
     notmuch_filenames_t *db_subdirs = NULL;
+    time_t stat_time;
     struct stat st;
     notmuch_bool_t is_maildir, new_directory;
     const char **tag;
@@ -239,6 +241,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 		 path, strerror (errno));
 	return NOTMUCH_STATUS_FILE_ERROR;
     }
+    stat_time = time (NULL);
 
     /* This is not an error since we may have recursed based on a
      * symlink to a regular file, not a directory, and we don't know
@@ -509,7 +512,22 @@ add_files_recursive (notmuch_database_t *notmuch,
 	notmuch_filenames_move_to_next (db_subdirs);
     }
 
-    if (! interrupted) {
+    /* If the directory's mtime is the same as the wall-clock time
+     * when we stat'ed the directory, we skip updating the mtime in
+     * the database because a message could be delivered later in this
+     * same second.  This may lead to unnecessary re-scans, but it
+     * avoids overlooking messages.
+     *
+     * XXX Bug workaround: If this is a new directory, we *must*
+     * update the mtime; otherwise the next run will see the 0 mtime
+     * and think this is still a new directory containing no files or
+     * subdirs (which is unsound in general).  If fs_mtime ==
+     * stat_time, we set the database mtime to a bogus (but non-zero!)
+     * value to force a rescan.
+     */
+    if (new_directory && fs_mtime == stat_time)
+	fs_mtime = 1;
+    if (! interrupted && fs_mtime != stat_time) {
 	status = notmuch_directory_set_mtime (directory, fs_mtime);
 	if (status && ret == NOTMUCH_STATUS_SUCCESS)
 	    ret = status;
-- 
1.7.5.1