dirstate: invalidate changes when parent-change fails
authorPierre-Yves David <pierre-yves.david@octobus.net>
Tue, 13 Dec 2022 11:39:44 +0100
changeset 49958 96e526fe5fb0
parent 49957 ff4df0954742
child 49959 376395868b7b
dirstate: invalidate changes when parent-change fails When an error occurs during changing parents, we should invalidate all dirstate modifications and reload the dirstate. This is currently done by a `unlock` callback on the `wlock`. To fix this anomaly, we start dealing with the error directly in the context manager and its potential nesting. The "hard" part is to make sure that, when the parent-change context are nested, we and higher level nesting do not continue to use the invalidated dirstate. We introduce dedicated code to enforce that.
mercurial/dirstate.py
--- a/mercurial/dirstate.py	Sat Jan 28 20:08:57 2023 +0100
+++ b/mercurial/dirstate.py	Tue Dec 13 11:39:44 2022 +0100
@@ -72,6 +72,9 @@
             msg = 'calling `%s` outside of a parentchange context'
             msg %= func.__name__
             raise error.ProgrammingError(msg)
+        if self._invalidated_context:
+            msg = 'calling `%s` after the dirstate was invalidated'
+            raise error.ProgrammingError(msg)
         return func(self, *args, **kwargs)
 
     return wrap
@@ -124,7 +127,11 @@
         self._dirty_tracked_set = False
         self._ui = ui
         self._filecache = {}
+        # nesting level of `parentchange` context
         self._parentwriters = 0
+        # True if the current dirstate changing operations have been
+        # invalidated (used to make sure all nested contexts have been exited)
+        self._invalidated_context = False
         self._filename = b'dirstate'
         self._filename_th = b'dirstate-tracked-hint'
         self._pendingfilename = b'%s.pending' % self._filename
@@ -151,14 +158,27 @@
         the incoherent dirstate won't be written when wlock is
         released.
         """
+        if self._invalidated_context:
+            msg = "trying to use an invalidated dirstate before it has reset"
+            raise error.ProgrammingError(msg)
         self._parentwriters += 1
-        yield
-        # Typically we want the "undo" step of a context manager in a
-        # finally block so it happens even when an exception
-        # occurs. In this case, however, we only want to decrement
-        # parentwriters if the code in the with statement exits
-        # normally, so we don't have a try/finally here on purpose.
-        self._parentwriters -= 1
+        try:
+            yield
+        except Exception:
+            self.invalidate()
+            raise
+        finally:
+            if self._parentwriters > 0:
+                if self._invalidated_context:
+                    # make sure we invalidate anything an upper context might
+                    # have changed.
+                    self.invalidate()
+                self._parentwriters -= 1
+                # The invalidation is complete once we exit the final context
+                # manager
+                if self._parentwriters <= 0:
+                    assert self._parentwriters == 0
+                    self._invalidated_context = False
 
     def pendingparentchange(self):
         """Returns true if the dirstate is in the middle of a set of changes
@@ -419,7 +439,7 @@
                 delattr(self, a)
         self._dirty = False
         self._dirty_tracked_set = False
-        self._parentwriters = 0
+        self._invalidated_context = self._parentwriters > 0
         self._origpl = None
 
     def copy(self, source, dest):