Tue May 22 23:39:49 BST 2012 Brian Warner <warner@lothar.com>

  * test_web.py: fix memory leak when run with --until-failure

  The Fake*Node classes in test/common.py were accumulating share data in
  a class-level dictionary, which persisted from one test run to the next.
  As a result, running test_web.py over and over (with trial's
  --until-failure feature) made this dictionary grow without bound,
  eventually running out of memory.

  This fix moves that dictionary into the FakeClient built fresh for each
  test, so it doesn't build up. It does the same thing for "file_types",
  which was much smaller but still lived at the class level.

  Closes #1729

diff --git a/src/allmydata/test/common.py b/src/allmydata/test/common.py
index 108633562bcb1cf531f5991d2342606582476d68..7fdbb7862f734570c9c5c869e5ba0730c743de4f 100644 (file)
--- a/src/allmydata/test/common.py
+++ b/src/allmydata/test/common.py
@@ -6,7 +6,7 @@ from twisted.python import failure
 from twisted.application import service
 from twisted.web.error import Error as WebError
 from foolscap.api import flushEventualQueue, fireEventually
-from allmydata import uri, dirnode, client
+from allmydata import uri, client
 from allmydata.introducer.server import IntroducerNode
 from allmydata.interfaces import IMutableFileNode, IImmutableFileNode,\
                                  NotEnoughSharesError, ICheckable, \
@@ -43,10 +43,10 @@ class FakeCHKFileNode:
     """I provide IImmutableFileNode, but all of my data is stored in a
     class-level dictionary."""
     implements(IImmutableFileNode)
-    all_contents = {}
 
-    def __init__(self, filecap):
+    def __init__(self, filecap, all_contents):
         precondition(isinstance(filecap, (uri.CHKFileURI, uri.LiteralFileURI)), filecap)
+        self.all_contents = all_contents
         self.my_uri = filecap
         self.storage_index = self.my_uri.get_storage_index()
 
@@ -165,10 +165,10 @@ def make_chk_file_cap(size):
 def make_chk_file_uri(size):
     return make_chk_file_cap(size).to_string()
 
-def create_chk_filenode(contents):
+def create_chk_filenode(contents, all_contents):
     filecap = make_chk_file_cap(len(contents))
-    n = FakeCHKFileNode(filecap)
-    FakeCHKFileNode.all_contents[filecap.to_string()] = contents
+    n = FakeCHKFileNode(filecap, all_contents)
+    all_contents[filecap.to_string()] = contents
     return n
 
 
@@ -178,11 +178,11 @@ class FakeMutableFileNode:
 
     implements(IMutableFileNode, ICheckable)
     MUTABLE_SIZELIMIT = 10000
-    all_contents = {}
-    file_types = {} # storage index => MDMF_VERSION or SDMF_VERSION
 
     def __init__(self, storage_broker, secret_holder,
-                 default_encoding_parameters, history):
+                 default_encoding_parameters, history, all_contents):
+        self.all_contents = all_contents
+        self.file_types = {} # storage index => MDMF_VERSION or SDMF_VERSION
         self.init_from_cap(make_mutable_file_cap())
         self._k = default_encoding_parameters['k']
         self._segsize = default_encoding_parameters['max_segment_size']
@@ -400,7 +400,7 @@ def make_verifier_uri():
     return uri.SSKVerifierURI(storage_index=os.urandom(16),
                               fingerprint=os.urandom(32)).to_string()
 
-def create_mutable_filenode(contents, mdmf=False):
+def create_mutable_filenode(contents, mdmf=False, all_contents=None):
     # XXX: All of these arguments are kind of stupid. 
     if mdmf:
         cap = make_mdmf_mutable_file_cap()
@@ -411,7 +411,8 @@ def create_mutable_filenode(contents, mdmf=False):
     encoding_params['k'] = 3
     encoding_params['max_segment_size'] = 128*1024
 
-    filenode = FakeMutableFileNode(None, None, encoding_params, None)
+    filenode = FakeMutableFileNode(None, None, encoding_params, None,
+                                   all_contents)
     filenode.init_from_cap(cap)
     if mdmf:
         filenode.create(MutableData(contents), version=MDMF_VERSION)
@@ -420,14 +421,6 @@ def create_mutable_filenode(contents, mdmf=False):
     return filenode
 
 
-class FakeDirectoryNode(dirnode.DirectoryNode):
-    """This offers IDirectoryNode, but uses a FakeMutableFileNode for the
-    backing store, so it doesn't go to the grid. The child data is still
-    encrypted and serialized, so this isn't useful for tests that want to
-    look inside the dirnodes and check their contents.
-    """
-    filenode_class = FakeMutableFileNode
-
 class LoggingServiceParent(service.MultiService):
     def log(self, *args, **kwargs):
         return log.msg(*args, **kwargs)

diff --git a/src/allmydata/test/test_web.py b/src/allmydata/test/test_web.py
index f398260975076a6032384b7a7f6f12ad6aee1fee..70014bfbfc732100df433ae487784430e39db89c 100644 (file)
--- a/src/allmydata/test/test_web.py
+++ b/src/allmydata/test/test_web.py
@@ -55,16 +55,17 @@ class FakeNodeMaker(NodeMaker):
         'max_segment_size':128*1024 # 1024=KiB
     }
     def _create_lit(self, cap):
-        return FakeCHKFileNode(cap)
+        return FakeCHKFileNode(cap, self.all_contents)
     def _create_immutable(self, cap):
-        return FakeCHKFileNode(cap)
+        return FakeCHKFileNode(cap, self.all_contents)
     def _create_mutable(self, cap):
-        return FakeMutableFileNode(None,
-                                   None,
-                                   self.encoding_params, None).init_from_cap(cap)
+        return FakeMutableFileNode(None, None,
+                                   self.encoding_params, None,
+                                   self.all_contents).init_from_cap(cap)
     def create_mutable_file(self, contents="", keysize=None,
                             version=SDMF_VERSION):
-        n = FakeMutableFileNode(None, None, self.encoding_params, None)
+        n = FakeMutableFileNode(None, None, self.encoding_params, None,
+                                self.all_contents)
         return n.create(contents, version=version)
 
 class FakeUploader(service.Service):
@@ -74,7 +75,7 @@ class FakeUploader(service.Service):
         d.addCallback(lambda size: uploadable.read(size))
         def _got_data(datav):
             data = "".join(datav)
-            n = create_chk_filenode(data)
+            n = create_chk_filenode(data, self.all_contents)
             results = upload.UploadResults()
             results.uri = n.get_uri()
             return results
@@ -158,6 +159,7 @@ class FakeClient(Client):
         # don't upcall to Client.__init__, since we only want to initialize a
         # minimal subset
         service.MultiService.__init__(self)
+        self.all_contents = {}
         self.nodeid = "fake_nodeid"
         self.nickname = "fake_nickname"
         self.introducer_furl = "None"
@@ -169,11 +171,13 @@ class FakeClient(Client):
         self.introducer_client = None
         self.history = FakeHistory()
         self.uploader = FakeUploader()
+        self.uploader.all_contents = self.all_contents
         self.uploader.setServiceParent(self)
         self.blacklist = None
         self.nodemaker = FakeNodeMaker(None, self._secret_holder, None,
                                        self.uploader, None,
                                        None, None, None)
+        self.nodemaker.all_contents = self.all_contents
         self.mutable_file_default = SDMF_VERSION
 
     def startService(self):
@@ -251,7 +255,7 @@ class WebMixin(object):
 
             _ign, n, self._bad_file_uri = self.makefile(3)
             # this uri should not be downloadable
-            del FakeCHKFileNode.all_contents[self._bad_file_uri]
+            del self.s.all_contents[self._bad_file_uri]
 
             rodir = res[5][1]
             self.public_root.set_uri(u"reedownlee", rodir.get_readonly_uri(),
@@ -278,14 +282,17 @@ class WebMixin(object):
         d.addCallback(_got_metadata)
         return d
 
+    def get_all_contents(self):
+        return self.s.all_contents
+
     def makefile(self, number):
         contents = "contents of file %s\n" % number
-        n = create_chk_filenode(contents)
+        n = create_chk_filenode(contents, self.get_all_contents())
         return contents, n, n.get_uri()
 
     def makefile_mutable(self, number, mdmf=False):
         contents = "contents of mutable file %s\n" % number
-        n = create_mutable_filenode(contents, mdmf)
+        n = create_mutable_filenode(contents, mdmf, self.s.all_contents)
         return contents, n, n.get_uri(), n.get_readonly_uri()
 
     def tearDown(self):
@@ -1982,7 +1989,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
         return d
 
     def failUnlessCHKURIHasContents(self, got_uri, contents):
-        self.failUnless(FakeCHKFileNode.all_contents[got_uri] == contents)
+        self.failUnless(self.get_all_contents()[got_uri] == contents)
 
     def test_POST_upload(self):
         d = self.POST(self.public_url + "/foo", t="upload",
@@ -2085,7 +2092,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
             self.failUnless(filecap.startswith("URI:SSK:"), filecap)
             self.filecap = filecap
             u = uri.WriteableSSKFileURI.init_from_string(filecap)
-            self.failUnlessIn(u.get_storage_index(), FakeMutableFileNode.all_contents)
+            self.failUnlessIn(u.get_storage_index(), self.get_all_contents())
             n = self.s.create_node_from_uri(filecap)
             return n.download_best_version()
         d.addCallback(_check)
@@ -2911,7 +2918,8 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
     def _create_immutable_children(self):
         contents, n, filecap1 = self.makefile(12)
         md1 = {"metakey1": "metavalue1"}
-        tnode = create_chk_filenode("immutable directory contents\n"*10)
+        tnode = create_chk_filenode("immutable directory contents\n"*10,
+                                    self.get_all_contents())
         dnode = DirectoryNode(tnode, None, None)
         assert not dnode.is_mutable()
         immdircap = dnode.get_uri()
@@ -3533,8 +3541,8 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
         d = self.PUT("/uri", file_contents)
         def _check(uri):
             assert isinstance(uri, str), uri
-            self.failUnlessIn(uri, FakeCHKFileNode.all_contents)
-            self.failUnlessReallyEqual(FakeCHKFileNode.all_contents[uri],
+            self.failUnlessIn(uri, self.get_all_contents())
+            self.failUnlessReallyEqual(self.get_all_contents()[uri],
                                        file_contents)
             return self.GET("/uri/%s" % uri)
         d.addCallback(_check)
@@ -3548,8 +3556,8 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
         d = self.PUT("/uri?mutable=false", file_contents)
         def _check(uri):
             assert isinstance(uri, str), uri
-            self.failUnlessIn(uri, FakeCHKFileNode.all_contents)
-            self.failUnlessReallyEqual(FakeCHKFileNode.all_contents[uri],
+            self.failUnlessIn(uri, self.get_all_contents())
+            self.failUnlessReallyEqual(self.get_all_contents()[uri],
                                        file_contents)
             return self.GET("/uri/%s" % uri)
         d.addCallback(_check)
@@ -3574,7 +3582,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
             self.failUnless(filecap.startswith("URI:SSK:"), filecap)
             self.filecap = filecap
             u = uri.WriteableSSKFileURI.init_from_string(filecap)
-            self.failUnlessIn(u.get_storage_index(), FakeMutableFileNode.all_contents)
+            self.failUnlessIn(u.get_storage_index(), self.get_all_contents())
             n = self.s.create_node_from_uri(filecap)
             return n.download_best_version()
         d.addCallback(_check1)