[PATCH STABLE] localrepo: clear filecache correctly on destroyed()

Hao Lian hao at fogcreek.com
Mon Nov 19 16:12:00 CST 2012


# HG changeset patch
# User Hao Lian <hao at fogcreek.com>
# Date 1353360901 18000
# Branch stable
# Node ID 0d053ca02a3d94e56634091690a9a58c8d833329
# Parent  35ba170c0f82dba18f0207ef4bd93216e6de8bbf
localrepo: clear filecache correctly on destroyed()

Assume R is a localrepo and P is a property of R decorated with storecache.
Prior to 9f94358, invalidate() enforced the invariant that P is a member of
R.__dict__ if and only if it were also mirrored in R's filecache.  Now, however,
P may be deleted from filecache without the corresponding object being deleted
in R.__dict__.  This allows the following bug to occur:

* P is deleted from R's filecache.
* The underlying file that P corresponds to is updated.
* Someone attempts to access P.
* Because the stale P is in __dict__, storecache returns stale P.

Prior to 9f94358, P would have been deleted from R's both __dict__ and
filecache, never just one of them.

This exact bug occurs during hg convert, which rolls back empty revisions when
converting Subversion repositories.  When that happens, _rollback() calls
destroyed(), which deletes the changelog object from filecache without deleting
it from __dict__.  As a result, the changelog becomes stale and len(changelog)
is one larger than the actual length of the changelog; as the conversion
progresses, hg convert (sometimes) errors out during an integrity check on the
changelog.

The fix is to simply undo the changes of 9f94358.  However, we still want
destroyed() to clear the filecache, which was the intent of that changeset.
That is accomplished by calling invalidate(), which (now) enforces the
invariant.  Since invalidate() calls invalidatecaches() anyway, we can simply
replace the call to invalidatecaches() with invalidate().

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1004,9 +1004,6 @@
                         self.sjoin('phaseroots'))
         self.invalidate()
 
-        # Discard all cache entries to force reloading everything.
-        self._filecache.clear()
-
         parentgone = (parents[0] not in self.changelog.nodemap or
                       parents[1] not in self.changelog.nodemap)
         if parentgone:
@@ -1075,6 +1072,9 @@
                 pass
         self.invalidatecaches()
 
+        # Discard all cache entries to force reloading everything.
+        self._filecache.clear()
+
     def _lock(self, lockname, wait, releasefn, acquirefn, desc):
         try:
             l = lock.lock(lockname, 0, releasefn, desc=desc)
@@ -1507,10 +1507,8 @@
         # head, refresh the tag cache, then immediately add a new head.
         # But I think doing it this way is necessary for the "instant
         # tag cache retrieval" case to work.
-        self.invalidatecaches()
+        self.invalidate()
 
-        # Discard all cache entries to force reloading everything.
-        self._filecache.clear()
 
     def walk(self, match, node=None):
         '''


More information about the Mercurial-devel mailing list