[PATCH manifest-leak-fix] lazymanifest: prevent leak when updating an entry more than once

Augie Fackler raf at durin42.com
Sat Apr 11 15:58:13 UTC 2015


# HG changeset patch
# User Augie Fackler <augie at google.com>
# Date 1428767781 14400
#      Sat Apr 11 11:56:21 2015 -0400
# Node ID 1ae719dda0da9f935e7fb49681c3b6467677df1d
# Parent  fbdbff1b486a5b9c12c0cebf4d172be90a2bb5f8
lazymanifest: prevent leak when updating an entry more than once

__setitem__ on the lazymanifest C type wasn't checking to see if a
line had previously been malloced before replacing it, leading to
leaks if files got updated multiple times in the course of a task.

I was able to reproduce the leak with this change to test-manifest.py:

diff --git a/tests/test-manifest.py b/tests/test-manifest.py
--- a/tests/test-manifest.py
+++ b/tests/test-manifest.py
@@ -456,6 +456,16 @@ class basemanifesttests(object):
                 ['a/b/c/bar.txt', 'a/b/c/foo.txt', 'a/b/d/ten.txt'],
                 m2.keys())

+    def testManifestSetItem(self):
+        m = self.parsemanifest('')
+        for x in range(3):
+            m['file%d' % x] = BIN_HASH_1
+        for x in range(3):
+            m['file%d' % x] = BIN_HASH_2
+        import time
+        time.sleep(4)
+
+

along with the commands:

 $ make local
 $ PYTHONPATH=. SILENT_BE_NOISY=1 python tests/test-manifest.py testmanifestdict.testManifestSetItem &
 $ sleep 4
 $ leaks $(jobs -p | tee /dev/stderr | awk '{print $3}')
 $ wait

in an interactive shell on OS X. As far as I can tell, it had to be an
interactive shell so that I could get the pid of the test run using
the jobs builtin. Prior to this change, I was leaking several strings,
and after this change leaks reports no leaks.

I thought there was a bug filed for this in bugzilla, but I can't find
it either in bugzilla or by searching my email.

diff --git a/mercurial/manifest.c b/mercurial/manifest.c
--- a/mercurial/manifest.c
+++ b/mercurial/manifest.c
@@ -440,6 +440,8 @@ static int internalsetitem(lazymanifest 
 		else {
 			if (self->lines[pos].deleted)
 				self->livelines++;
+			if (self->lines[pos].from_malloc)
+				free(self->lines[pos].start);
 			start = pos;
 			goto finish;
 		}


More information about the Mercurial-devel mailing list