manifest/revlog: do not let the revlog cache mutable objects
authorBenoit Boissinot <benoit.boissinot@ens-lyon.org>
Fri, 04 Sep 2009 10:47:55 +0200
changeset 9420 d0db168136dc
parent 9419 3516a4e877c1
child 9421 c8e4dc218aaf
manifest/revlog: do not let the revlog cache mutable objects If a buffer of an mutable object is passed to revlog.addrevision(), the revlog will happily store it in its cache. Later when the revlog reuses the cached entry, if the manifest modified the object in-between, all kind of bugs appears. We fix it by: - passing immutable objects to addrevision() if they are already available - only storing the text in the cache if it's of str type Then we can remove the conversion of the cache entry to str() during retrieval. That was probably just there hiding the bug for the common cases but not really fixing it.
mercurial/manifest.py
mercurial/revlog.py
--- a/mercurial/manifest.py	Thu Sep 03 21:40:45 2009 +0200
+++ b/mercurial/manifest.py	Fri Sep 04 10:47:55 2009 +0200
@@ -132,9 +132,9 @@
             # if this is changed to support newlines in filenames,
             # be sure to check the templates/ dir again (especially *-raw.tmpl)
             hex, flags = revlog.hex, map.flags
-            text = ["%s\000%s%s\n" % (f, hex(map[f]), flags(f))
-                    for f in files]
-            arraytext = array.array('c', "".join(text))
+            text = ''.join("%s\000%s%s\n" % (f, hex(map[f]), flags(f))
+                           for f in files)
+            arraytext = array.array('c', text)
             cachedelta = None
         else:
             added, removed = changed
@@ -190,9 +190,9 @@
             if p1 != self.tip():
                 cachedelta = None
             arraytext = addlist
+            text = buffer(arraytext)
 
-        n = self.addrevision(buffer(arraytext), transaction, link,
-                             p1, p2, cachedelta)
+        n = self.addrevision(text, transaction, link, p1, p2, cachedelta)
         self._mancache = (n, map, arraytext)
 
         return n
--- a/mercurial/revlog.py	Thu Sep 03 21:40:45 2009 +0200
+++ b/mercurial/revlog.py	Fri Sep 04 10:47:55 2009 +0200
@@ -973,7 +973,7 @@
         if node == nullid:
             return ""
         if self._cache and self._cache[0] == node:
-            return str(self._cache[2])
+            return self._cache[2]
 
         # look up what we need to read
         text = None
@@ -988,7 +988,7 @@
         # do we have useful data cached?
         if self._cache and self._cache[1] >= base and self._cache[1] < rev:
             base = self._cache[1]
-            text = str(self._cache[2])
+            text = self._cache[2]
 
         self._loadindex(base, rev + 1)
         self._chunkraw(base, rev)
@@ -1111,7 +1111,8 @@
             ifh.write(data[1])
             self.checkinlinesize(transaction, ifh)
 
-        self._cache = (node, curr, text)
+        if type(text) == str: # only accept immutable objects
+            self._cache = (node, curr, text)
         return node
 
     def ancestor(self, a, b):