Commits

Robert Brewer committed eb5ca91

Fixed a refleak in _cpwsgi.py: errors in start_response weren't closing the app and releasing the serving. As part of investigating and fixing that, added a new 'lib/gctools.py' module with a GCRoot class which any app can mount and call, and made the test suite call and verify its output in (nearly) every test class.

  • Participants
  • Parent commits 7dedb7e

Comments (0)

Files changed (12)

File py2/cherrypy/_cpwsgi.py

             raise
         r = _cherrypy.serving.response
         self.iter_response = iter(r.body)
-        self.write = start_response(r.output_status, r.header_list)
+        try:
+            self.write = start_response(r.output_status, r.header_list)
+        except:
+            self.close()
+            raise
     
     def __iter__(self):
         return self

File py2/cherrypy/lib/gctools.py

+import gc
+import inspect
+
+from cherrypy import _cprequest
+
+
+class ReferrerTree(object):
+    """An object which gathers all referrers of an object to a given depth."""
+
+    peek_length = 40
+
+    def __init__(self, ignore=None, maxdepth=2, maxparents=10):
+        self.ignore = ignore or []
+        self.ignore.append(inspect.currentframe().f_back)
+        self.maxdepth = maxdepth
+        self.maxparents = maxparents
+
+    def ascend(self, obj, depth=1):
+        """Return a nested list containing referrers of the given object."""
+        depth += 1
+        parents = []
+
+        # Gather all referrers in one step to minimize
+        # cascading references due to repr() logic.
+        refs = gc.get_referrers(obj)
+        self.ignore.append(refs)
+        if len(refs) > self.maxparents:
+            return [("[%s referrers]" % len(refs), [])]
+
+        for parent in refs:
+            if inspect.isframe(parent) and parent.f_code is self.ascend.__code__:
+                continue
+            if parent in self.ignore:
+                continue
+            if depth <= self.maxdepth:
+                parents.append((parent, self.ascend(parent, depth)))
+            else:
+                parents.append((parent, []))
+
+        return parents
+
+    def peek(self, s):
+        """Return s, restricted to a sane length."""
+        if len(s) > (self.peek_length + 3):
+            half = self.peek_length // 2
+            return s[:half] + '...' + s[-half:]
+        else:
+            return s
+
+    def _format(self, obj, descend=True):
+        """Return a string representation of a single object."""
+        if inspect.isframe(obj):
+            filename, lineno, func, context, index = inspect.getframeinfo(obj)
+            return "<frame of function '%s'>" % func
+
+        if not descend:
+            return self.peek(repr(obj))
+
+        if isinstance(obj, dict):
+            return "{" + ", ".join(["%s: %s" % (self._format(k, descend=False),
+                                                self._format(v, descend=False))
+                                    for k, v in obj.items()]) + "}"
+        elif isinstance(obj, list):
+            return "[" + ", ".join([self._format(item, descend=False)
+                                    for item in obj]) + "]"
+        elif isinstance(obj, tuple):
+            return "(" + ", ".join([self._format(item, descend=False)
+                                    for item in obj]) + ")"
+
+        r = self.peek(repr(obj))
+        if isinstance(obj, (str, int, float)):
+            return r
+        return "%s: %s" % (type(obj), r)
+
+    def format(self, tree):
+        """Return a list of string reprs from a nested list of referrers."""
+        output = []
+        def ascend(branch, depth=1):
+            for parent, grandparents in branch:
+                output.append(("    " * depth) + self._format(parent))
+                if grandparents:
+                    ascend(grandparents, depth + 1)
+        ascend(tree)
+        return output
+
+
+def get_instances(cls):
+    return [x for x in gc.get_objects() if isinstance(x, cls)]
+
+
+class GCRoot(object):
+    """A CherryPy page handler for testing reference leaks."""
+
+    classes = [(_cprequest.Request, 2, 2,
+                "Should be 1 in this request thread and 1 in the main thread."),
+               (_cprequest.Response, 2, 2,
+                "Should be 1 in this request thread and 1 in the main thread."),
+               ]
+
+    def index(self):
+        return "Hello, world!"
+    index.exposed = True
+
+    def stats(self):
+        output = ["Statistics:"]
+        
+        # gc_collect isn't perfectly synchronous, because it may
+        # break reference cycles that then take time to fully
+        # finalize. Call it twice and hope for the best.
+        gc.collect()
+        unreachable = gc.collect()
+        if unreachable:
+            output.append("\n%s unreachable objects:" % unreachable)
+            trash = {}
+            for x in gc.garbage:
+                trash[type(x)] = trash.get(type(x), 0) + 1
+            trash = [(v, k) for k, v in trash.items()]
+            trash.sort()
+            for pair in trash:
+                output.append("    " + repr(pair))
+        
+        # Check declared classes to verify uncollected instances.
+        # These don't have to be part of a cycle; they can be
+        # any objects that have unanticipated referrers that keep
+        # them from being collected.
+        for cls, minobj, maxobj, msg in self.classes:
+            objs = get_instances(cls)
+            lenobj = len(objs)
+            if lenobj < minobj or lenobj > maxobj:
+                if minobj == maxobj:
+                    output.append(
+                        "\nExpected %s %r references, got %s." %
+                        (minobj, cls, lenobj))
+                else:
+                    output.append(
+                        "\nExpected %s to %s %r references, got %s." %
+                        (minobj, maxobj, cls, lenobj))
+                for obj in objs:
+                    output.append("\nReferrers for %s:" % repr(obj))
+                    t = ReferrerTree(ignore=[objs], maxdepth=3)
+                    tree = t.ascend(obj)
+                    output.extend(t.format(tree))
+        
+        return "\n".join(output)
+    stats.exposed = True
+

File py2/cherrypy/test/helper.py

 import cherrypy
 from cherrypy._cpcompat import basestring, copyitems, HTTPSConnection, ntob
 from cherrypy.lib import httputil
+from cherrypy.lib import gctools
 from cherrypy.lib.reprconf import unrepr
 from cherrypy.test import webtest
 
             cherrypy.tree = cherrypy._cptree.Tree()
             cherrypy.server.httpserver = None
             cls.setup_server()
+            # Add a resource for verifying there are no refleaks
+            # to *every* test class.
+            cherrypy.tree.mount(gctools.GCRoot(), '/gc')
+            cls.do_gc_test = True
             supervisor.start(cls.__module__)
 
         cls.supervisor = supervisor
             cls.supervisor.stop()
     teardown_class = classmethod(teardown_class)
     
+    do_gc_test = False
+    
+    def test_gc(self):
+        if self.do_gc_test:
+            self.getPage("/gc/stats")
+            self.assertBody("Statistics:")
+    
     def prefix(self):
         return self.script_name.rstrip("/")
     

File py2/cherrypy/test/test_core.py

                     # Python2's SimpleCookie.__setitem__ won't take unicode keys.
                     cherrypy.response.cookie[str(name)] = cookie.value
 
-
         def append_headers(header_list, debug=False):
             if debug:
                 cherrypy.log(
         class MultiHeader(Test):
             
             @cherrypy.tools.append_headers(header_list=[
-                ('WWW-Authenticate', 'Negotiate'),
-                ('WWW-Authenticate', 'Basic realm="foo"'),
+                (ntob('WWW-Authenticate'), ntob('Negotiate')),
+                (ntob('WWW-Authenticate'), ntob('Basic realm="foo"')),
                 ])
             def header_list(self):
                 pass
         self.assertStatus(200)
         self.assertBody("Mr. and Mrs. Watson")
 
+
+class ErrorTests(helper.CPWebCase):
+
+    def setup_server():
+        def break_header():
+            # Add a header after finalize that is invalid
+            cherrypy.serving.response.header_list.append((2, 3))
+        cherrypy.tools.break_header = cherrypy.Tool('on_end_resource', break_header)
+        
+        class Root:
+            def index(self):
+                return "hello"
+            index.exposed = True
+            
+            def start_response_error(self):
+                return "salud!"
+            start_response_error._cp_config = {'tools.break_header.on': True}
+        root = Root()
+        
+        cherrypy.tree.mount(root)
+    setup_server = staticmethod(setup_server)
+
+    def test_start_response_error(self):
+        self.getPage("/start_response_error")
+        self.assertStatus(500)
+        self.assertInBody("TypeError: WSGI response header key 2 is not a byte string.")

File py2/cherrypy/test/test_refleaks.py

 """Tests for refleaks."""
 
-import gc
 from cherrypy._cpcompat import HTTPConnection, HTTPSConnection, ntob
 import threading
 
 import cherrypy
-from cherrypy import _cprequest
 
 
 data = object()
 
-def get_instances(cls):
-    return [x for x in gc.get_objects() if isinstance(x, cls)]
-
 
 from cherrypy.test import helper
 
                 cherrypy.request.thing = data
                 return "Hello world!"
             index.exposed = True
-            
-            def gc_stats(self):
-                output = ["Statistics:"]
-                
-                # Uncollectable garbage
-                
-                # gc_collect isn't perfectly synchronous, because it may
-                # break reference cycles that then take time to fully
-                # finalize. Call it twice and hope for the best.
-                gc.collect()
-                unreachable = gc.collect()
-                if unreachable:
-                    output.append("\n%s unreachable objects:" % unreachable)
-                    trash = {}
-                    for x in gc.garbage:
-                        trash[type(x)] = trash.get(type(x), 0) + 1
-                    trash = [(v, k) for k, v in trash.items()]
-                    trash.sort()
-                    for pair in trash:
-                        output.append("    " + repr(pair))
-                
-                # Request references
-                reqs = get_instances(_cprequest.Request)
-                lenreqs = len(reqs)
-                if lenreqs < 2:
-                    output.append("\nMissing Request reference. Should be 1 in "
-                                  "this request thread and 1 in the main thread.")
-                elif lenreqs > 2:
-                    output.append("\nToo many Request references (%r)." % lenreqs)
-                    for req in reqs:
-                        output.append("Referrers for %s:" % repr(req))
-                        for ref in gc.get_referrers(req):
-                            if ref is not reqs:
-                                output.append("    %s" % repr(ref))
-                
-                # Response references
-                resps = get_instances(_cprequest.Response)
-                lenresps = len(resps)
-                if lenresps < 2:
-                    output.append("\nMissing Response reference. Should be 1 in "
-                                  "this request thread and 1 in the main thread.")
-                elif lenresps > 2:
-                    output.append("\nToo many Response references (%r)." % lenresps)
-                    for resp in resps:
-                        output.append("Referrers for %s:" % repr(resp))
-                        for ref in gc.get_referrers(resp):
-                            if ref is not resps:
-                                output.append("    %s" % repr(ref))
-                
-                return "\n".join(output)
-            gc_stats.exposed = True
         
         cherrypy.tree.mount(Root())
     setup_server = staticmethod(setup_server)
-
     
     def test_threadlocal_garbage(self):
         success = []
             t.join()
         
         self.assertEqual(len(success), ITERATIONS)
-        
-        self.getPage("/gc_stats")
-        self.assertBody("Statistics:")
 

File py2/cherrypy/test/test_states.py

 
     def setUp(self):
         cherrypy.server.socket_timeout = 0.1
+        self.do_gc_test = False
 
     def test_0_NormalStateFlow(self):
         engine.stop()

File py3/cherrypy/_cpwsgi.py

             raise
         r = _cherrypy.serving.response
         self.iter_response = iter(r.body)
-        self.write = start_response(r.output_status, r.header_list)
+        try:
+            self.write = start_response(r.output_status, r.header_list)
+        except:
+            self.close()
+            raise
     
     def __iter__(self):
         return self

File py3/cherrypy/lib/gctools.py

+import gc
+import inspect
+
+from cherrypy import _cprequest
+
+
+class ReferrerTree(object):
+    """An object which gathers all referrers of an object to a given depth."""
+
+    peek_length = 40
+
+    def __init__(self, ignore=None, maxdepth=2, maxparents=10):
+        self.ignore = ignore or []
+        self.ignore.append(inspect.currentframe().f_back)
+        self.maxdepth = maxdepth
+        self.maxparents = maxparents
+
+    def ascend(self, obj, depth=1):
+        """Return a nested list containing referrers of the given object."""
+        depth += 1
+        parents = []
+
+        # Gather all referrers in one step to minimize
+        # cascading references due to repr() logic.
+        refs = gc.get_referrers(obj)
+        self.ignore.append(refs)
+        if len(refs) > self.maxparents:
+            return [("[%s referrers]" % len(refs), [])]
+
+        for parent in refs:
+            if inspect.isframe(parent) and parent.f_code is self.ascend.__code__:
+                continue
+            if parent in self.ignore:
+                continue
+            if depth <= self.maxdepth:
+                parents.append((parent, self.ascend(parent, depth)))
+            else:
+                parents.append((parent, []))
+
+        return parents
+
+    def peek(self, s):
+        """Return s, restricted to a sane length."""
+        if len(s) > (self.peek_length + 3):
+            half = self.peek_length // 2
+            return s[:half] + '...' + s[-half:]
+        else:
+            return s
+
+    def _format(self, obj, descend=True):
+        """Return a string representation of a single object."""
+        if inspect.isframe(obj):
+            filename, lineno, func, context, index = inspect.getframeinfo(obj)
+            return "<frame of function '%s'>" % func
+
+        if not descend:
+            return self.peek(repr(obj))
+
+        if isinstance(obj, dict):
+            return "{" + ", ".join(["%s: %s" % (self._format(k, descend=False),
+                                                self._format(v, descend=False))
+                                    for k, v in obj.items()]) + "}"
+        elif isinstance(obj, list):
+            return "[" + ", ".join([self._format(item, descend=False)
+                                    for item in obj]) + "]"
+        elif isinstance(obj, tuple):
+            return "(" + ", ".join([self._format(item, descend=False)
+                                    for item in obj]) + ")"
+
+        r = self.peek(repr(obj))
+        if isinstance(obj, (str, int, float)):
+            return r
+        return "%s: %s" % (type(obj), r)
+
+    def format(self, tree):
+        """Return a list of string reprs from a nested list of referrers."""
+        output = []
+        def ascend(branch, depth=1):
+            for parent, grandparents in branch:
+                output.append(("    " * depth) + self._format(parent))
+                if grandparents:
+                    ascend(grandparents, depth + 1)
+        ascend(tree)
+        return output
+
+
+def get_instances(cls):
+    return [x for x in gc.get_objects() if isinstance(x, cls)]
+
+
+class GCRoot(object):
+    """A CherryPy page handler for testing reference leaks."""
+
+    classes = [(_cprequest.Request, 2, 2,
+                "Should be 1 in this request thread and 1 in the main thread."),
+               (_cprequest.Response, 2, 2,
+                "Should be 1 in this request thread and 1 in the main thread."),
+               ]
+
+    def index(self):
+        return "Hello, world!"
+    index.exposed = True
+
+    def stats(self):
+        output = ["Statistics:"]
+        
+        # gc_collect isn't perfectly synchronous, because it may
+        # break reference cycles that then take time to fully
+        # finalize. Call it twice and hope for the best.
+        gc.collect()
+        unreachable = gc.collect()
+        if unreachable:
+            output.append("\n%s unreachable objects:" % unreachable)
+            trash = {}
+            for x in gc.garbage:
+                trash[type(x)] = trash.get(type(x), 0) + 1
+            trash = [(v, k) for k, v in trash.items()]
+            trash.sort()
+            for pair in trash:
+                output.append("    " + repr(pair))
+        
+        # Check declared classes to verify uncollected instances.
+        # These don't have to be part of a cycle; they can be
+        # any objects that have unanticipated referrers that keep
+        # them from being collected.
+        for cls, minobj, maxobj, msg in self.classes:
+            objs = get_instances(cls)
+            lenobj = len(objs)
+            if lenobj < minobj or lenobj > maxobj:
+                if minobj == maxobj:
+                    output.append(
+                        "\nExpected %s %r references, got %s." %
+                        (minobj, cls, lenobj))
+                else:
+                    output.append(
+                        "\nExpected %s to %s %r references, got %s." %
+                        (minobj, maxobj, cls, lenobj))
+                for obj in objs:
+                    output.append("\nReferrers for %s:" % repr(obj))
+                    t = ReferrerTree(ignore=[objs], maxdepth=3)
+                    tree = t.ascend(obj)
+                    output.extend(t.format(tree))
+        
+        return "\n".join(output)
+    stats.exposed = True
+

File py3/cherrypy/test/helper.py

 import cherrypy
 from cherrypy._cpcompat import basestring, copyitems, HTTPSConnection, ntob
 from cherrypy.lib import httputil
+from cherrypy.lib import gctools
 from cherrypy.lib.reprconf import unrepr
 from cherrypy.test import webtest
 
             cherrypy.tree = cherrypy._cptree.Tree()
             cherrypy.server.httpserver = None
             cls.setup_server()
+            # Add a resource for verifying there are no refleaks
+            # to *every* test class.
+            cherrypy.tree.mount(gctools.GCRoot(), '/gc')
+            cls.do_gc_test = True
             supervisor.start(cls.__module__)
 
         cls.supervisor = supervisor
             cls.supervisor.stop()
     teardown_class = classmethod(teardown_class)
     
+    do_gc_test = False
+    
+    def test_gc(self):
+        if self.do_gc_test:
+            self.getPage("/gc/stats")
+            self.assertBody("Statistics:")
+    
     def prefix(self):
         return self.script_name.rstrip("/")
     

File py3/cherrypy/test/test_core.py

                     # Python2's SimpleCookie.__setitem__ won't take unicode keys.
                     cherrypy.response.cookie[str(name)] = cookie.value
 
-
         def append_headers(header_list, debug=False):
             if debug:
                 cherrypy.log(
         class MultiHeader(Test):
             
             @cherrypy.tools.append_headers(header_list=[
-                ('WWW-Authenticate', 'Negotiate'),
-                ('WWW-Authenticate', 'Basic realm="foo"'),
+                (ntob('WWW-Authenticate'), ntob('Negotiate')),
+                (ntob('WWW-Authenticate'), ntob('Basic realm="foo"')),
                 ])
             def header_list(self):
                 pass
         self.assertStatus(200)
         self.assertBody("Mr. and Mrs. Watson")
 
+
+class ErrorTests(helper.CPWebCase):
+
+    def setup_server():
+        def break_header():
+            # Add a header after finalize that is invalid
+            cherrypy.serving.response.header_list.append((2, 3))
+        cherrypy.tools.break_header = cherrypy.Tool('on_end_resource', break_header)
+        
+        class Root:
+            def index(self):
+                return "hello"
+            index.exposed = True
+            
+            def start_response_error(self):
+                return "salud!"
+            start_response_error._cp_config = {'tools.break_header.on': True}
+        root = Root()
+        
+        cherrypy.tree.mount(root)
+    setup_server = staticmethod(setup_server)
+
+    def test_start_response_error(self):
+        self.getPage("/start_response_error")
+        self.assertStatus(500)
+        self.assertInBody("TypeError: WSGI response header key 2 is not a byte string.")

File py3/cherrypy/test/test_refleaks.py

 """Tests for refleaks."""
 
-import gc
 from cherrypy._cpcompat import HTTPConnection, HTTPSConnection, ntob
 import threading
 
 import cherrypy
-from cherrypy import _cprequest
 
 
 data = object()
 
-def get_instances(cls):
-    return [x for x in gc.get_objects() if isinstance(x, cls)]
-
 
 from cherrypy.test import helper
 
                 cherrypy.request.thing = data
                 return "Hello world!"
             index.exposed = True
-            
-            def gc_stats(self):
-                output = ["Statistics:"]
-                
-                # Uncollectable garbage
-                
-                # gc_collect isn't perfectly synchronous, because it may
-                # break reference cycles that then take time to fully
-                # finalize. Call it twice and hope for the best.
-                gc.collect()
-                unreachable = gc.collect()
-                if unreachable:
-                    output.append("\n%s unreachable objects:" % unreachable)
-                    trash = {}
-                    for x in gc.garbage:
-                        trash[type(x)] = trash.get(type(x), 0) + 1
-                    trash = [(v, k) for k, v in trash.items()]
-                    trash.sort()
-                    for pair in trash:
-                        output.append("    " + repr(pair))
-                
-                # Request references
-                reqs = get_instances(_cprequest.Request)
-                lenreqs = len(reqs)
-                if lenreqs < 2:
-                    output.append("\nMissing Request reference. Should be 1 in "
-                                  "this request thread and 1 in the main thread.")
-                elif lenreqs > 2:
-                    output.append("\nToo many Request references (%r)." % lenreqs)
-                    for req in reqs:
-                        output.append("Referrers for %s:" % repr(req))
-                        for ref in gc.get_referrers(req):
-                            if ref is not reqs:
-                                output.append("    %s" % repr(ref))
-                
-                # Response references
-                resps = get_instances(_cprequest.Response)
-                lenresps = len(resps)
-                if lenresps < 2:
-                    output.append("\nMissing Response reference. Should be 1 in "
-                                  "this request thread and 1 in the main thread.")
-                elif lenresps > 2:
-                    output.append("\nToo many Response references (%r)." % lenresps)
-                    for resp in resps:
-                        output.append("Referrers for %s:" % repr(resp))
-                        for ref in gc.get_referrers(resp):
-                            if ref is not resps:
-                                output.append("    %s" % repr(ref))
-                
-                return "\n".join(output)
-            gc_stats.exposed = True
         
         cherrypy.tree.mount(Root())
     setup_server = staticmethod(setup_server)
-
     
     def test_threadlocal_garbage(self):
         success = []
             t.join()
         
         self.assertEqual(len(success), ITERATIONS)
-        
-        self.getPage("/gc_stats")
-        self.assertBody("Statistics:")
 

File py3/cherrypy/test/test_states.py

 
     def setUp(self):
         cherrypy.server.socket_timeout = 0.1
+        self.do_gc_test = False
 
     def test_0_NormalStateFlow(self):
         engine.stop()