changeset 32816:1b25c648d5b7

fsmonitor: don't write out state if identity has changed (issue5581) Inspired by the dirstate fix in dc7efa2826e4, this should fix any race conditions with the fsmonitor state changing from underneath. Since we now grab the wlock for any non-invalidate writes, the only situation this appears to happen in is with a concurrent invalidation. Test that.
author Siddharth Agarwal <sid0@fb.com>
date Mon, 12 Jun 2017 15:34:31 -0700
parents 15e85dded933
children e962c70c0aad
files hgext/fsmonitor/state.py mercurial/util.py tests/test-dirstate-race.t
diffstat 3 files changed, 51 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/hgext/fsmonitor/state.py	Mon Jun 12 15:34:31 2017 -0700
+++ b/hgext/fsmonitor/state.py	Mon Jun 12 15:34:31 2017 -0700
@@ -13,7 +13,10 @@
 import struct
 
 from mercurial.i18n import _
-from mercurial import pathutil
+from mercurial import (
+    pathutil,
+    util,
+)
 
 _version = 4
 _versionformat = ">I"
@@ -24,6 +27,7 @@
         self._ui = repo.ui
         self._rootdir = pathutil.normasprefix(repo.root)
         self._lastclock = None
+        self._identity = util.filestat(None)
 
         self.mode = self._ui.config('fsmonitor', 'mode', default='on')
         self.walk_on_invalidate = self._ui.configbool(
@@ -35,10 +39,13 @@
         try:
             file = self._vfs('fsmonitor.state', 'rb')
         except IOError as inst:
+            self._identity = util.filestat(None)
             if inst.errno != errno.ENOENT:
                 raise
             return None, None, None
 
+        self._identity = util.filestat.fromfp(file)
+
         versionbytes = file.read(4)
         if len(versionbytes) < 4:
             self._ui.log(
@@ -90,8 +97,16 @@
             self.invalidate()
             return
 
+        # Read the identity from the file on disk rather than from the open file
+        # pointer below, because the latter is actually a brand new file.
+        identity = util.filestat.frompath(self._vfs.join('fsmonitor.state'))
+        if identity != self._identity:
+            self._ui.debug('skip updating fsmonitor.state: identity mismatch\n')
+            return
+
         try:
-            file = self._vfs('fsmonitor.state', 'wb', atomictemp=True)
+            file = self._vfs('fsmonitor.state', 'wb', atomictemp=True,
+                checkambig=True)
         except (IOError, OSError):
             self._ui.warn(_("warning: unable to write out fsmonitor state\n"))
             return
@@ -111,6 +126,7 @@
         except OSError as inst:
             if inst.errno != errno.ENOENT:
                 raise
+        self._identity = util.filestat(None)
 
     def setlastclock(self, clock):
         self._lastclock = clock
--- a/mercurial/util.py	Mon Jun 12 15:34:31 2017 -0700
+++ b/mercurial/util.py	Mon Jun 12 15:34:31 2017 -0700
@@ -1519,6 +1519,11 @@
             stat = None
         return cls(stat)
 
+    @classmethod
+    def fromfp(cls, fp):
+        stat = os.fstat(fp.fileno())
+        return cls(stat)
+
     __hash__ = object.__hash__
 
     def __eq__(self, old):
--- a/tests/test-dirstate-race.t	Mon Jun 12 15:34:31 2017 -0700
+++ b/tests/test-dirstate-race.t	Mon Jun 12 15:34:31 2017 -0700
@@ -160,6 +160,34 @@
 
   $ rm b
 
+#if fsmonitor
+
+Create fsmonitor state.
+
+  $ hg status
+  $ f --type .hg/fsmonitor.state
+  .hg/fsmonitor.state: file
+
+Test that invalidating fsmonitor state in the middle (which doesn't require the
+wlock) causes the fsmonitor update to be skipped.
+hg debugrebuilddirstate ensures that the dirstaterace hook will be called, but
+it also invalidates the fsmonitor state. So back it up and restore it.
+
+  $ mv .hg/fsmonitor.state .hg/fsmonitor.state.tmp
+  $ hg debugrebuilddirstate
+  $ mv .hg/fsmonitor.state.tmp .hg/fsmonitor.state
+
+  $ cat > $TESTTMP/dirstaterace.sh <<EOF
+  > rm .hg/fsmonitor.state
+  > EOF
+
+  $ hg status --config extensions.dirstaterace=$TESTTMP/dirstaterace.py --debug
+  skip updating fsmonitor.state: identity mismatch
+  $ f .hg/fsmonitor.state
+  .hg/fsmonitor.state: file not found
+
+#endif
+
 Set up a rebase situation for issue5581.
 
   $ echo c2 > a