lazymanifest: prevent leak when updating an entry more than once
authorAugie Fackler <augie@google.com>
Sat, 11 Apr 2015 11:56:21 -0400
changeset 24710 909ee6b2a024
parent 24709 69154e0ae384
child 24711 ab6e3729747e
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.
mercurial/manifest.c
--- a/mercurial/manifest.c	Mon Apr 13 07:42:25 2015 -0500
+++ b/mercurial/manifest.c	Sat Apr 11 11:56:21 2015 -0400
@@ -440,6 +440,8 @@
 		else {
 			if (self->lines[pos].deleted)
 				self->livelines++;
+			if (self->lines[pos].from_malloc)
+				free(self->lines[pos].start);
 			start = pos;
 			goto finish;
 		}