Bug 761564 - Memory leaks in Mozmill Python code which keeps referenc…

…es around and never completely releases the Mozmill object. r=jhammel, r=ctalbert

mozmill/docs/api_example.py

@@ -7,15 +7,18 @@
 # you have to have some sort of test
 import os
 import tempfile
+
+import mozmill
+
 test = """var test_something = function() { }"""
 fd, path = tempfile.mkstemp()
 os.write(fd, test)
 os.close(fd)
 
 # now to do our thing: basic run
-import mozmill
 m = mozmill.MozMill.create()
-results = m.run(dict(path=path))
+m.run(dict(path=path))
+results = m.finish()
 
 # there should be one passing test
 passes = 1
@@ -29,10 +32,11 @@
 # this is how you use a handler
 # let's try the logging handler:
 from mozmill.logger import LoggerListener
+
 logger = LoggerListener()
-m = mozmill.MozMill.create(results=results, handlers=(logger,))
-results = m.run(dict(path=path))
-results.finish((logger,))
+m = mozmill.MozMill.create(handlers=(logger,))
+m.run(dict(path=path))
+results = m.finish()
 
 # now there should be two
 passes *= 2

mozmill/mozmill/__init__.py

@@ -42,6 +42,7 @@
 
 class TestResults(object):
     """Class to accumulate test results and other information."""
+
     def __init__(self):
         # application information
         self.appinfo = {}
@@ -64,15 +65,6 @@ def events(self):
         """Events, the MozMill class will dispatch to."""
         return {'mozmill.endTest': self.endTest_listener}
 
-    def finish(self, handlers, fatal=False):
-        """Do the final reporting and such."""
-        self.endtime = datetime.utcnow()
-
-        # handle stop events
-        for handler in handlers:
-            if hasattr(handler, 'stop'):
-                handler.stop(self, fatal)
-
     ### event listener
     def endTest_listener(self, test):
         """Add current test result to results."""
@@ -97,13 +89,14 @@ class MozMill(object):
     You should use MozMill as follows:
 
         m = MozMill(...)
-        results = m.run(tests)
-        results.finish()
+        m.run(tests)
+        m.run(other_tests)
+        results = m.finish()
     """
 
     @classmethod
-    def create(cls, results=None, jsbridge_timeout=JSBRIDGE_TIMEOUT,
-               handlers=(), app='firefox', profile_args=None,
+    def create(cls, jsbridge_timeout=JSBRIDGE_TIMEOUT,
+               handlers=None, app='firefox', profile_args=None,
                runner_args=None):
 
         jsbridge_port = jsbridge.find_port()
@@ -135,19 +128,18 @@ def create(cls, results=None, jsbridge_timeout=JSBRIDGE_TIMEOUT,
         runner = runner_class.create(**runner_args)
 
         # create a mozmill
-        return cls(runner, jsbridge_port, results=results,
-                   jsbridge_timeout=jsbridge_timeout, handlers=handlers)
+        return cls(runner, jsbridge_port, jsbridge_timeout=jsbridge_timeout,
+                   handlers=handlers)
 
-    def __init__(self, runner, jsbridge_port, results=None,
-                 jsbridge_timeout=JSBRIDGE_TIMEOUT, handlers=()):
+    def __init__(self, runner, jsbridge_port,
+                 jsbridge_timeout=JSBRIDGE_TIMEOUT, handlers=None):
         """Constructor of the Mozmill class.
 
         Arguments:
         runner -- The MozRunner instance to run the application
         jsbridge_port -- The port the jsbridge server is running on
 
         Keyword arguments:
-        results -- A TestResults instance to accumulate results
         jsbridge_timeout -- How long to wait without a jsbridge communication
         handlers -- pluggable event handlers
 
@@ -165,7 +157,7 @@ def __init__(self, runner, jsbridge_port, results=None,
         self.bridge = self.back_channel = None
 
         # Report data will end up here
-        self.results = results or TestResults()
+        self.results = TestResults()
 
         # persisted data
         self.persisted = {}
@@ -174,11 +166,38 @@ def __init__(self, runner, jsbridge_port, results=None,
         self.shutdownMode = {}
         self.endRunnerCalled = False
 
-        # setup event listeners
-        self.global_listeners = []
+        # list of listeners and handlers
         self.listeners = []
-        # dict of listeners by event type
-        self.listener_dict = {}
+        self.listener_dict = {}  # by event type
+        self.global_listeners = []
+        self.handlers = []
+
+        # setup event handlers and register listeners
+        self.setup_listeners()
+
+        handlers = handlers or list()
+        handlers.append(self.results)
+        self.setup_handlers(handlers)
+
+        # disable the crashreporter
+        os.environ['MOZ_CRASHREPORTER_NO_REPORT'] = '1'
+
+    ### methods for event listeners
+
+    def setup_handlers(self, handlers):
+        for handler in handlers:
+            self.handlers.append(handler)
+
+            # make the mozmill instance available to the handler
+            handler.mozmill = self
+
+            if hasattr(handler, 'events'):
+                for event, method in handler.events().items():
+                    self.add_listener(method, eventType=event)
+            if hasattr(handler, '__call__'):
+                self.add_global_listener(handler)
+
+    def setup_listeners(self):
         self.add_listener(self.endRunner_listener,
                           eventType='mozmill.endRunner')
         self.add_listener(self.frameworkFail_listener,
@@ -192,25 +211,6 @@ def __init__(self, runner, jsbridge_port, results=None,
         self.add_listener(self.userShutdown_listener,
                           eventType='mozmill.userShutdown')
 
-        # add listeners for event handlers
-        self.handlers = [self.results]
-        self.handlers.extend(handlers)
-        for handler in self.handlers:
-
-            # make the mozmill instance available to the handler
-            handler.mozmill = self
-
-            if hasattr(handler, 'events'):
-                for event, method in handler.events().items():
-                    self.add_listener(method, eventType=event)
-            if hasattr(handler, '__call__'):
-                self.add_global_listener(handler)
-
-        # disable the crashreporter
-        os.environ['MOZ_CRASHREPORTER_NO_REPORT'] = '1'
-
-    ### methods for event listeners
-
     def add_listener(self, callback, eventType):
         self.listener_dict.setdefault(eventType, []).append(callback)
         self.listeners.append((callback, {'eventType': eventType}))
@@ -347,7 +347,8 @@ def run_test_file(self, frame, path, name=None):
                 # if there is not a next test,
                 # throw the error up the chain
                 raise
-            frame = self.run_test_file(self.start_runner(), path, nextTest)
+            frame = self.run_test_file(self.start_runner(),
+                                       path, nextTest)
 
         return frame
 
@@ -417,8 +418,6 @@ def run(self, tests, restart=False):
             self.running_test = None
             self.stop()
 
-        return self.results
-
     def get_appinfo(self, bridge):
         """Collect application specific information."""
         app_info = { }
@@ -435,6 +434,27 @@ def get_appinfo(self, bridge):
 
     ### methods for shutting down and cleanup
 
+    def finish(self, fatal=False):
+        """Do the final reporting and such."""
+        self.results.endtime = datetime.utcnow()
+
+        # handle stop events
+        for handler in self.handlers:
+            if hasattr(handler, 'stop'):
+                handler.stop(self.results, fatal)
+
+            # setup_handlers() sets a reference to the mozmill object.
+            # It's not necessary anymore and has to be reset to avoid
+            # circular references
+            handler.mozmill = None
+
+        self.listeners = []
+        self.listener_dict = {}
+        self.global_listeners = []
+        self.handlers = []
+
+        return self.results
+
     def report_disconnect(self, message=None):
         message = message or 'Disconnect Error: Application unexpectedly closed'
 
@@ -474,7 +494,7 @@ def stop_runner(self, timeout=10):
             raise Exception('client process shutdown unsuccessful')
 
     def stop(self):
-        """Cleanup and invoking of final handlers."""
+        """Cleanup after a run"""
 
         # ensure you have the application info for the case
         # of no tests: https://bugzilla.mozilla.org/show_bug.cgi?id=751866
@@ -483,11 +503,15 @@ def stop(self):
             self.start_runner()
             self.stop_runner()
 
-        # close the bridge and back channel
+        # stop the back channel and bridge
         if self.back_channel:
             self.back_channel.close()
             self.bridge.close()
 
+        # release objects
+        self.back_channel = None
+        self.bridge = None
+
         # cleanup
         if self.runner is not None:
             self.runner.cleanup()
@@ -728,8 +752,7 @@ def run(self):
             exception_type, exception, tb = sys.exc_info()
 
         # do whatever reporting you're going to do
-        results = mozmill.results
-        results.finish(self.event_handlers, fatal=exception is not None)
+        results = mozmill.finish(fatal=exception is not None)
 
         # exit on bad stuff happen
         if exception:

mozmill/mozmill/logger.py

@@ -24,6 +24,7 @@ def __init__(self, log_file=None, console_level="INFO", file_level="INFO",
                  format="pprint-color", debug=False, console_stream=sys.stdout):
         self.format = format
         self.debug = debug
+        self.mozmill = None
 
         template = "%(levelname)s | %(message)s"
 

mutt/mutt/mutt.py

@@ -179,8 +179,8 @@ def test_all_js(tests, options):
     level = "DEBUG" if options.verbose else "INFO"
     logger = LoggerListener(console_level=level)
 
+    m = mozmill.MozMill.create(handlers=[logger])
     try:
-        m = mozmill.MozMill.create(handlers=[logger])
         m.run(tests, options.restart)
     except:
         exception_type, exception, tb = sys.exc_info()

mutt/mutt/tests/python/test_bug690154.py

@@ -23,8 +23,10 @@ def make_test(self):
     def test_JSON_structure(self):
         passes = 1
         path = self.make_test()
+
         m = mozmill.MozMill.create()
-        results = m.run([dict(path=path)])
+        m.run([dict(path=path)])
+        results = m.finish()
 
         # no modules pass
         self.assertFalse(results.passes)

mutt/mutt/tests/python/test_endTest.py

@@ -15,7 +15,8 @@ def test_modules(self):
             strict=False)
 
         m = mozmill.MozMill.create()
-        results = m.run(manifest.active_tests())
+        m.run(manifest.active_tests())
+        results = m.finish()
 
         # From the first test, there is one passing test
         self.assertEqual(len(results.passes), 3, "Passes should match")

mutt/mutt/tests/python/test_loggerlistener.py

@@ -1,6 +1,5 @@
 from cStringIO import StringIO
 import os
-import sys
 import unittest
 import tempfile
 
@@ -27,9 +26,9 @@ def test_logger_listener(self):
         logger_info = LoggerListener(console_level="INFO", console_stream=info_data)
         logger_debug = LoggerListener(console_level="DEBUG", console_stream=debug_data)
 
-        m = mozmill.MozMill.create(handlers=(logger_info, logger_debug))
-        results = m.run(tests)
-        results.finish((logger_info, logger_debug))
+        m = mozmill.MozMill.create(handlers=[logger_info, logger_debug])
+        m.run(tests)
+        m.finish()
 
         assert "TEST-START" in debug_data.getvalue()
         assert "TEST-PASS" in debug_data.getvalue()

mutt/mutt/tests/python/test_references.py

@@ -0,0 +1,47 @@
+from cStringIO import StringIO
+import os
+import sys
+import tempfile
+import unittest
+
+import mozmill
+from mozmill.logger import LoggerListener
+
+
+class ModuleTest(unittest.TestCase):
+    def make_test(self):
+        """make an example test to run"""
+        test = """var test_something = function() {}"""
+        fd, path = tempfile.mkstemp()
+        os.write(fd, test)
+        os.close(fd)
+
+        return path
+
+    def do_test(self, relative_test_path):
+        testpath = os.path.join(os.path.dirname(os.path.abspath(__file__)),
+                                relative_test_path)
+        tests = [{'path': testpath}]
+
+        info_data= StringIO()
+        logger = LoggerListener(console_level="DEBUG", console_stream=info_data)
+
+        m = mozmill.MozMill.create(handlers=[logger])
+        m.run(tests)
+        results = m.finish()
+
+        self.assertEqual(sys.getrefcount(logger), 2,
+                         "Only a single reference to the logger exists")
+        self.assertEqual(sys.getrefcount(m), 2,
+                         "Only a single reference to mozmill exists")
+
+        return results
+
+    def test_modules(self):
+        results = self.do_test(self.make_test())
+
+        self.assertEqual(sys.getrefcount(results), 2,
+                         "Only a single reference to results exists")
+
+if __name__ == '__main__':
+    unittest.main()

mutt/mutt/tests/python/testapi.py

@@ -20,8 +20,11 @@ def make_test(self):
     def test_api(self):
         passes = 1
         path = self.make_test()
+
         m = mozmill.MozMill.create()
-        results = m.run([dict(path=path)])
+        m.run([dict(path=path)])
+        results = m.finish()
+
         self.assertTrue(len(results.passes) == passes)
 
 if __name__ == '__main__':

mutt/mutt/tests/python/testmultiplerun.py

@@ -20,9 +20,12 @@ def make_test(self):
     def test_runtwice(self):
         passes = 2
         path = self.make_test()
+
         m = mozmill.MozMill.create()
         m.run([dict(path=path)])
-        results = m.run([dict(path=path)])
+        m.run([dict(path=path)])
+        results = m.finish()
+
         self.assertTrue(len(results.passes) == passes)
 
 if __name__ == '__main__':