[Lldb-commits] [lldb] r250467 - Factor the execution of the test method into a separate function to ensure that any exceptions that are thrown go out of scope and no longer hold references to SB objects that need to be freed before teardown.

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Thu Oct 15 17:39:55 2015
New Revision: 250467

URL: http://llvm.org/viewvc/llvm-project?rev=250467&view=rev
Log:
Factor the execution of the test method into a separate function to ensure that any exceptions that are thrown go out of scope and no longer hold references to SB objects that need to be freed before teardown.

Differential Revision: http://reviews.llvm.org/D13788

Modified:
    lldb/trunk/source/Target/Target.cpp
    lldb/trunk/test/lldbtest.py
    lldb/trunk/test/unittest2/case.py

Modified: lldb/trunk/source/Target/Target.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=250467&r1=250466&r2=250467&view=diff
==============================================================================
--- lldb/trunk/source/Target/Target.cpp (original)
+++ lldb/trunk/source/Target/Target.cpp Thu Oct 15 17:39:55 2015
@@ -1276,7 +1276,7 @@ Target::ModuleAdded (const ModuleList& m
 void
 Target::ModuleRemoved (const ModuleList& module_list, const ModuleSP &module_sp)
 {
-    // A module is being added to this target for the first time
+    // A module is being removed from this target.
     if (m_valid)
     {
         ModuleList my_module_list;

Modified: lldb/trunk/test/lldbtest.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/lldbtest.py?rev=250467&r1=250466&r2=250467&view=diff
==============================================================================
--- lldb/trunk/test/lldbtest.py (original)
+++ lldb/trunk/test/lldbtest.py Thu Oct 15 17:39:55 2015
@@ -32,6 +32,7 @@ $
 """
 
 import abc
+import gc
 import glob
 import os, sys, traceback
 import os.path
@@ -2544,6 +2545,11 @@ class TestBase(Base):
         #import traceback
         #traceback.print_stack()
 
+        # Ensure all the references to SB objects have gone away so that we can
+        # be sure that all test-specific resources have been freed before we
+        # attempt to delete the targets.
+        gc.collect()
+
         # Delete the target(s) from the debugger as a general cleanup step.
         # This includes terminating the process for each target, if any.
         # We'd like to reuse the debugger for our next test without incurring

Modified: lldb/trunk/test/unittest2/case.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/unittest2/case.py?rev=250467&r1=250466&r2=250467&view=diff
==============================================================================
--- lldb/trunk/test/unittest2/case.py (original)
+++ lldb/trunk/test/unittest2/case.py Thu Oct 15 17:39:55 2015
@@ -353,32 +353,7 @@ class TestCase(unittest.TestCase):
             except Exception:
                 result.addError(self, sys.exc_info())
             else:
-                try:
-                    testMethod()
-                except self.failureException:
-                    result.addFailure(self, sys.exc_info())
-                except _ExpectedFailure, e:
-                    addExpectedFailure = getattr(result, 'addExpectedFailure', None)
-                    if addExpectedFailure is not None:
-                        addExpectedFailure(self, e.exc_info, e.bugnumber)
-                    else: 
-                        warnings.warn("Use of a TestResult without an addExpectedFailure method is deprecated", 
-                                      DeprecationWarning)
-                        result.addSuccess(self)
-                except _UnexpectedSuccess, x:
-                    addUnexpectedSuccess = getattr(result, 'addUnexpectedSuccess', None)
-                    if addUnexpectedSuccess is not None:
-                        addUnexpectedSuccess(self, x.bugnumber)
-                    else:
-                        warnings.warn("Use of a TestResult without an addUnexpectedSuccess method is deprecated", 
-                                      DeprecationWarning)
-                        result.addFailure(self, sys.exc_info())
-                except SkipTest, e:
-                    self._addSkip(result, str(e))
-                except Exception:
-                    result.addError(self, sys.exc_info())
-                else:
-                    success = True
+                success = self.runMethod(testMethod, result)
 
                 try:
                     self.tearDown()
@@ -399,6 +374,42 @@ class TestCase(unittest.TestCase):
                 if stopTestRun is not None:
                     stopTestRun()
 
+    def runMethod(self, testMethod, result):
+        """Runs the test method and catches any exception that might be thrown.
+
+        This is factored out of TestCase.run() to ensure that any exception
+        thrown during the test goes out of scope before tearDown.  Otherwise, an
+        exception could hold references to Python objects that are bound to
+        SB objects and prevent them from being deleted in time.
+        """
+        try:
+            testMethod()
+        except self.failureException:
+            result.addFailure(self, sys.exc_info())
+        except _ExpectedFailure, e:
+            addExpectedFailure = getattr(result, 'addExpectedFailure', None)
+            if addExpectedFailure is not None:
+                addExpectedFailure(self, e.exc_info, e.bugnumber)
+            else:
+                warnings.warn("Use of a TestResult without an addExpectedFailure method is deprecated",
+                              DeprecationWarning)
+                result.addSuccess(self)
+        except _UnexpectedSuccess, x:
+            addUnexpectedSuccess = getattr(result, 'addUnexpectedSuccess', None)
+            if addUnexpectedSuccess is not None:
+                addUnexpectedSuccess(self, x.bugnumber)
+            else:
+                warnings.warn("Use of a TestResult without an addUnexpectedSuccess method is deprecated",
+                              DeprecationWarning)
+                result.addFailure(self, sys.exc_info())
+        except SkipTest, e:
+            self._addSkip(result, str(e))
+        except Exception:
+            result.addError(self, sys.exc_info())
+        else:
+            return True
+        return False
+
     def doCleanups(self):
         """Execute all cleanup functions. Normally called for you after
         tearDown."""