Merge lp:~jameinel/bzr/2.0.4-494406-serve-memory-leak into lp:bzr/2.0

[John A Meinel]

[John A Meinel (jameinel)]

I managed to track down the memory leak on server-side operations to bzrlib.bencode.Encoder.

It turns out that Encoder was using '__del__' as its cleanup function, when it should have been '__dealloc__'. I don't know if __del__ ever gets called for Pyrex objects, but I do know that __dealloc__ always gets called. (And doesn't cause problems with GC cycles, as it gets put into the tp_dealloc slot.)

Anyway, this one-line patch gets rid of the memory leak as near as I can tell. (At least cycling on 'bzr commit' no longer leaks memory.)

It could also have a fair impact on codehostings 'lp-serve'. As that would be encoding a *lot* of bytes to send over the wire. This memory was also "dark" memory, in that it was allocated directly using 'malloc()' and thus not seen by Meliae. I'm not positive, but I think for every byte that 'bzr lp-serve' would send over the wire, it would get wrapped by a 'bencode' operation, which means that every byte sent out gets permanently leaked in memory. (Maybe we don't serve bulk data through a bencode wrapper.) At least, all of the response tuples are certainly encoded.

It doesn't affect my 'memory reduction' tests that I used for 2.1.0b3, because those were operating locally.

[Andrew Bennetts (spiv)]

My understanding of Pyrex is that it doesn't treat __del__ as a special method at all. This change makes perfectly good sense, good catch!

review: Approve

=== modified file 'NEWS'
--- NEWS	2009-12-15 22:04:24 +0000
+++ NEWS	2009-12-18 22:33:13 +0000
@@ -8,7 +8,7 @@
 bzr 2.0.4 (not released yet)
 ############################
 
-:Codename: template
+:Codename:
 :2.0.4: ???
 
 Compatibility Breaks
@@ -25,6 +25,12 @@
   exporting over the network significantly faster (54min => 9min in one
   case). (John Arbash Meinel, #343218)
 
+* ``bzr serve`` no longer slowly leaks memory. The compiled
+  ``bzrlib.bencode.Encoder()`` class was using ``__del__`` to cleanup and
+  free resources, and it should have been using ``__dealloc__``.
+  This will likely have an impact on any other process that is serving for
+  an extended period of time.  (John Arbash Meinel, #494406)
+
 Improvements
 ************
 
 
=== modified file 'bzrlib/_bencode_pyx.pyx'
--- bzrlib/_bencode_pyx.pyx	2009-06-05 01:48:32 +0000
+++ bzrlib/_bencode_pyx.pyx	2009-12-18 22:33:13 +0000
@@ -261,7 +261,7 @@
         self.maxsize = maxsize
         self.tail = p
 
-    def __del__(self):
+    def __dealloc__(self):
         free(self.buffer)
         self.buffer = NULL
         self.maxsize = 0