bug#13149: 24.3.50; Emacs thinks file was changed outside Emacs, but it was not

[Paul Eggert]

On 01/17/13 02:32, Dmitry Gutov wrote:
> I think we need this to work without requiring the user to customize a 
> variable.

I agree.  But I would rather avoid having Emacs assume
that file system time stamps can be off by several seconds,
since that'll cause Emacs to miss changes that it should report.

Does the following patch help?  It should be applied anyway,
since it closes a minor race even on non-buggy file systems.
I'm hoping that it helps to work around your bug by using
fstat both times, instead of fstat in one place and stat
in another, since the file system bug seems to be that
fstat and stat disagree.

=== modified file 'src/ChangeLog'
--- src/ChangeLog       2013-01-17 06:29:40 +0000
+++ src/ChangeLog       2013-01-17 21:09:26 +0000
@@ -1,3 +1,11 @@
+2013-01-17  Paul Eggert  <eggert@cs.ucla.edu>
+
+       Close a race when statting and reading files (Bug#13149).
+       * fileio.c (Finsert_file_contents): Use open+fstat, not stat+open.
+       This avoids a race if the file is renamed between stat and open.
+       Also, perhaps it works around a file system bug that happen in
+       virtualized environments based on MS-Windows hosts.
+
 2013-01-17  Dmitry Antipov  <dmantipov@yandex.ru>
 
        * lisp.h (toplevel): Add comment about using Lisp_Save_Value

=== modified file 'src/fileio.c'
--- src/fileio.c        2013-01-17 06:29:40 +0000
+++ src/fileio.c        2013-01-17 21:09:26 +0000
@@ -3492,7 +3492,6 @@
   (Lisp_Object filename, Lisp_Object visit, Lisp_Object beg, Lisp_Object end, 
Lisp_Object replace)
 {
   struct stat st;
-  int file_status;
   EMACS_TIME mtime;
   int fd;
   ptrdiff_t inserted = 0;
@@ -3554,26 +3553,9 @@
   orig_filename = filename;
   filename = ENCODE_FILE (filename);
 
-  fd = -1;
-
-#ifdef WINDOWSNT
-  {
-    Lisp_Object tem = Vw32_get_true_file_attributes;
-
-    /* Tell stat to use expensive method to get accurate info.  */
-    Vw32_get_true_file_attributes = Qt;
-    file_status = stat (SSDATA (filename), &st);
-    Vw32_get_true_file_attributes = tem;
-  }
-#else
-  file_status = stat (SSDATA (filename), &st);
-#endif /* WINDOWSNT */
-
-  if (file_status == 0)
-    mtime = get_stat_mtime (&st);
-  else
+  fd = emacs_open (SSDATA (filename), O_RDONLY, 0);
+  if (fd < 0)
     {
-    badopen:
       save_errno = errno;
       if (NILP (visit))
        report_file_error ("Opening input file", Fcons (orig_filename, Qnil));
@@ -3585,6 +3567,17 @@
       goto notfound;
     }
 
+  /* Replacement should preserve point as it preserves markers.  */
+  if (!NILP (replace))
+    record_unwind_protect (restore_point_unwind, Fpoint_marker ());
+
+  record_unwind_protect (close_file_unwind, make_number (fd));
+
+  if (fstat (fd, &st) != 0)
+    report_file_error ("Getting input file status",
+                      Fcons (orig_filename, Qnil));
+  mtime = get_stat_mtime (&st);
+
   /* This code will need to be changed in order to work on named
      pipes, and it's probably just not worth it.  So we should at
      least signal an error.  */
@@ -3600,17 +3593,6 @@
                  build_string ("not a regular file"), orig_filename);
     }
 
-  if (fd < 0)
-    if ((fd = emacs_open (SSDATA (filename), O_RDONLY, 0)) < 0)
-      goto badopen;
-
-  /* Replacement should preserve point as it preserves markers.  */
-  if (!NILP (replace))
-    record_unwind_protect (restore_point_unwind, Fpoint_marker ());
-
-  record_unwind_protect (close_file_unwind, make_number (fd));
-
-
   if (!NILP (visit))
     {
       if (!NILP (beg) || !NILP (end))