Merge pull request #2006 from MSeifert04/fix-1965

nicoddemus · web-flow · commit fc304b8b44dd · 2016-11-09T19:19:40.000-02:00
Fix memory leak with pytest.raises by using weakref
diff --git a/AUTHORS b/AUTHORS
@@ -99,6 +99,7 @@ mbyt
 Michael Aquilina
 Michael Birtwell
 Michael Droettboom
+Michael Seifert
 Mike Lundy
 Nicolas Delaby
 Oleg Pidsadnyi
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -12,6 +12,12 @@
 * When loading plugins, import errors which contain non-ascii messages are now properly handled in Python 2 (`#1998`_).
   Thanks `@nicoddemus`_ for the PR.
 
+* Fixed cyclic reference when ``pytest.raises`` is used in context-manager form (`#1965`_). Also as a
+  result of this fix, ``sys.exc_info()`` is left empty in both context-manager and function call usages.
+  Previously, ``sys.exc_info`` would contain the exception caught by the context manager,
+  even when the expected exception occurred.
+  Thanks `@MSeifert04`_ for the report and the PR.
+
 * Fixed false-positives warnings from assertion rewrite hook for modules that were rewritten but
   were later marked explicitly by ``pytest.register_assert_rewrite``
   or implicitly as a plugin (`#2005`_).
@@ -36,12 +42,14 @@
 
 .. _@adborden: https://github.com/adborden
 .. _@cwitty: https://github.com/cwitty
-.. _@okulynyak: https://github.com/okulynyak
-.. _@matclab: https://github.com/matclab
-.. _@gdyuldin: https://github.com/gdyuldin
 .. _@d_b_w: https://github.com/d_b_w
+.. _@gdyuldin: https://github.com/gdyuldin
+.. _@matclab: https://github.com/matclab
+.. _@MSeifert04: https://github.com/MSeifert04
+.. _@okulynyak: https://github.com/okulynyak
 
 .. _#442: https://github.com/pytest-dev/pytest/issues/442
+.. _#1965: https://github.com/pytest-dev/pytest/issues/1965
 .. _#1976: https://github.com/pytest-dev/pytest/issues/1976
 .. _#1984: https://github.com/pytest-dev/pytest/issues/1984
 .. _#1998: https://github.com/pytest-dev/pytest/issues/1998
diff --git a/_pytest/_code/code.py b/_pytest/_code/code.py
@@ -1,6 +1,7 @@
 import sys
 from inspect import CO_VARARGS, CO_VARKEYWORDS
 import re
+from weakref import ref
 
 import py
 builtin_repr = repr
@@ -230,7 +231,7 @@ def ishidden(self):
                 return False
 
         if py.builtin.callable(tbh):
-            return tbh(self._excinfo)
+            return tbh(None if self._excinfo is None else self._excinfo())
         else:
             return tbh
 
@@ -370,7 +371,7 @@ def __init__(self, tup=None, exprinfo=None):
         #: the exception type name
         self.typename = self.type.__name__
         #: the exception traceback (_pytest._code.Traceback instance)
-        self.traceback = _pytest._code.Traceback(self.tb, excinfo=self)
+        self.traceback = _pytest._code.Traceback(self.tb, excinfo=ref(self))
 
     def __repr__(self):
         return "<ExceptionInfo %s tblen=%d>" % (self.typename, len(self.traceback))
diff --git a/_pytest/python.py b/_pytest/python.py
@@ -1237,7 +1237,11 @@ def __exit__(self, *tp):
                 exc_type, value, traceback = tp
                 tp = exc_type, exc_type(value), traceback
         self.excinfo.__init__(tp)
-        return issubclass(self.excinfo.type, self.expected_exception)
+        suppress_exception = issubclass(self.excinfo.type, self.expected_exception)
+        if sys.version_info[0] == 2 and suppress_exception:
+            sys.exc_clear()
+        return suppress_exception
+
 
 # builtin pytest.approx helper
 
diff --git a/testing/python/raises.py b/testing/python/raises.py
@@ -1,4 +1,6 @@
 import pytest
+import sys
+
 
 class TestRaises:
     def test_raises(self):
@@ -96,3 +98,31 @@ def test_custom_raise_message(self):
             assert e.msg == message
         else:
             assert False, "Expected pytest.raises.Exception"
+
+    @pytest.mark.parametrize('method', ['function', 'with'])
+    def test_raises_cyclic_reference(self, method):
+        """
+        Ensure pytest.raises does not leave a reference cycle (#1965).
+        """
+        import gc
+
+        class T(object):
+            def __call__(self):
+                raise ValueError
+
+        t = T()
+        if method == 'function':
+            pytest.raises(ValueError, t)
+        else:
+            with pytest.raises(ValueError):
+                t()
+
+        # ensure both forms of pytest.raises don't leave exceptions in sys.exc_info()
+        assert sys.exc_info() == (None, None, None)
+
+        del t
+
+        # ensure the t instance is not stuck in a cyclic reference
+        for o in gc.get_objects():
+            assert type(o) is not T
+
