Exception state lives too long in 3.0

Lib/test/test_exceptions.py

 # Python test set -- part 5, built-in exceptions
 
 import os
 import sys
 import unittest
 import pickle
+import weakref
 
 from test.test_support import TESTFN, unlink, run_unittest
 
 # XXX This is not really enough, each *operation* should be tested!
 
 class ExceptionTests(unittest.TestCase):
 
     def raise_catch(self, exc, excname):
         try:
             raise exc("spam")
@@ skipping 376 matching lines @@
         self.assertRaises(RuntimeError, g)
 
     def testUnicodeStrUsage(self):
         # Make sure both instances and classes have a str and unicode
         # representation.
         self.failUnless(str(Exception))
         self.failUnless(str(Exception))
         self.failUnless(str(Exception('a')))
         self.failUnless(str(Exception('a')))
 
-    def testExceptionCleanup(self):
-        # Make sure "except V as N" exceptions are cleaned up properly
+    def testExceptionCleanupNames(self):

[108414_gmail.com, 2008/05/02 23:19:14]

This is a test.

+        # Make sure the local variable bound to the exception instance by
+        # an "except" statement is only visible inside the except block.
 
         try:
             raise Exception()
         except Exception as e:
             self.failUnless(e)
             del e
         self.failIf('e' in locals())

[jhylton, 2008/05/06 12:59:48]

This test isn't too obvious.  The del e seems like to should guarantee that 'e'
is not in locals().  Why would it be in locals()?

[Antoine Pitrou, 2008/05/06 15:19:41]

On 2008/05/06 12:59:48, jhylton wrote:
> This test isn't too obvious.  The del e seems like to should guarantee that
'e'
> is not in locals().  Why would it be in locals()?

Honestly I don't know. This test is not part of the patch, I've just made sure
the comment is a bit more precise since "exceptions are cleaned up" is a bit
vague :-)

+
+    def testExceptionCleanupState(self):
+        # Make sure exception state is cleaned up as soon as the except
+        # block is left. See #2507
+
+        class MyException(Exception):
+            def __init__(self, obj):
+                self.obj = obj
+        class MyObj:
+            pass
+
+        def inner_raising_func():
+            # Create some references in exception value and traceback
+            local_ref = obj
+            raise MyException(obj)

[jhylton, 2008/05/06 12:59:48]

Are both references necessary?  Will the test fail if only one of them is
created?

[Antoine Pitrou, 2008/05/06 15:19:41]

On 2008/05/06 12:59:48, jhylton wrote:
> Are both references necessary?  Will the test fail if only one of them is
> created?

One of the references should be located in the exception traceback (via the
locals dictionary), the other in the exception value. This makes the test more
comprehensive than with only one reference. Obviously it will still pass if I
remove one of the references.

+
+        obj = MyObj()
+        wr = weakref.ref(obj)
+        try:
+            inner_raising_func()
+        except MyException as e:
+            pass
+        obj = None

[Jeffrey Yasskin, 2008/05/03 19:20:06]

Comment that this line exists to kill the reference to MyObj. Alternately, use:
  del obj
  self.assertEqual(wr(), None)
Which also avoids the need for the message parameter.

Just for future reference, str(obj) is probably better than "%s"%obj.

+        obj = wr()
+        self.failUnless(obj is None, "%s" % obj)
 
 
 def test_main():
     run_unittest(ExceptionTests)
 
 if __name__ == '__main__':
     unittest.main()