Fix ref cycles in TestCase.assertRaises() (python#193)

vstinner · web-flow · commit bbd3cf8f1ef1 · 2017-03-28T00:56:28.000+02:00
bpo-23890: unittest.TestCase.assertRaises() now manually breaks a
reference cycle to not keep objects alive longer than expected.
diff --git a/Lib/unittest/case.py b/Lib/unittest/case.py
@@ -153,28 +153,32 @@ def handle(self, name, args, kwargs):
         If args is not empty, call a callable passing positional and keyword
         arguments.
         """
-        if not _is_subtype(self.expected, self._base_type):
-            raise TypeError('%s() arg 1 must be %s' %
-                            (name, self._base_type_str))
-        if args and args[0] is None:
-            warnings.warn("callable is None",
-                          DeprecationWarning, 3)
-            args = ()
-        if not args:
-            self.msg = kwargs.pop('msg', None)
-            if kwargs:
-                warnings.warn('%r is an invalid keyword argument for '
-                              'this function' % next(iter(kwargs)),
-                              DeprecationWarning, 3)
-            return self
-
-        callable_obj, *args = args
         try:
-            self.obj_name = callable_obj.__name__
-        except AttributeError:
-            self.obj_name = str(callable_obj)
-        with self:
-            callable_obj(*args, **kwargs)
+            if not _is_subtype(self.expected, self._base_type):
+                raise TypeError('%s() arg 1 must be %s' %
+                                (name, self._base_type_str))
+            if args and args[0] is None:
+                warnings.warn("callable is None",
+                              DeprecationWarning, 3)
+                args = ()
+            if not args:
+                self.msg = kwargs.pop('msg', None)
+                if kwargs:
+                    warnings.warn('%r is an invalid keyword argument for '
+                                  'this function' % next(iter(kwargs)),
+                                  DeprecationWarning, 3)
+                return self
+
+            callable_obj, *args = args
+            try:
+                self.obj_name = callable_obj.__name__
+            except AttributeError:
+                self.obj_name = str(callable_obj)
+            with self:
+                callable_obj(*args, **kwargs)
+        finally:
+            # bpo-23890: manually break a reference cycle
+            self = None
 
 
 class _AssertRaisesContext(_AssertRaisesBaseContext):
@@ -725,7 +729,11 @@ def assertRaises(self, expected_exception, *args, **kwargs):
                self.assertEqual(the_exception.error_code, 3)
         """
         context = _AssertRaisesContext(expected_exception, self)
-        return context.handle('assertRaises', args, kwargs)
+        try:
+            return context.handle('assertRaises', args, kwargs)
+        finally:
+            # bpo-23890: manually break a reference cycle
+            context = None
 
     def assertWarns(self, expected_warning, *args, **kwargs):
         """Fail unless a warning of class warnClass is triggered
diff --git a/Lib/unittest/test/test_case.py b/Lib/unittest/test/test_case.py
@@ -1273,6 +1273,19 @@ def testAssertRaisesNoExceptionType(self):
         with self.assertRaises(TypeError):
             self.assertRaises((ValueError, object))
 
+    def testAssertRaisesRefcount(self):
+        # bpo-23890: assertRaises() must not keep objects alive longer
+        # than expected
+        def func() :
+            try:
+                raise ValueError
+            except ValueError:
+                raise ValueError
+
+        refcount = sys.getrefcount(func)
+        self.assertRaises(ValueError, func)
+        self.assertEqual(refcount, sys.getrefcount(func))
+
     def testAssertRaisesRegex(self):
         class ExceptionMock(Exception):
             pass
diff --git a/Misc/NEWS b/Misc/NEWS
@@ -291,6 +291,9 @@ Extension Modules
 Library
 -------
 
+- bpo-23890: unittest.TestCase.assertRaises() now manually breaks a reference
+  cycle to not keep objects alive longer than expected.
+
 - bpo-29901: The zipapp module now supports general path-like objects, not
   just pathlib.Path.
 
