amccarth created this revision.
amccarth added a reviewer: tfiala.
amccarth added a subscriber: lldb-commits.
This solves problems in tearDown of individual test cases, which were
especially noticeable on Windows.
When a test throws an exception, the exception retains references to the
functions in the stack, which in term keeps their local variables alive. When
those locals are SB proxy objects, cleanup doesn't proceed as it should because
they keep LLDB objects alive that should have been orphaned.
By moving the running of the test method to a separate function, we ensure that
the exception goes out of scope before we attempt tearDown, and the
gc.collect() makes sure the orphaned objects are properly destroyed before the
framework attempts to delete the target.
Without this fix, a test case that throws might cause the shared module list to
retain an open handle to the inferior, even after the target is deleted. This
affects the state of subsequent test cases in the same test. This was
especially noticeable on Windows in tests like TestTargetAPI, because the open
handle prevented make from deleting the inferior after the failed test case and
the next test case from re-building the inferior. Thus nearly all tests cases
would fail because of a problem in just one.
http://reviews.llvm.org/D13788
Files:
source/Target/Target.cpp
test/lldbtest.py
test/unittest2/case.py
Index: test/unittest2/case.py
===================================================================
--- test/unittest2/case.py
+++ test/unittest2/case.py
@@ -353,32 +353,7 @@
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 @@
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."""
Index: test/lldbtest.py
===================================================================
--- test/lldbtest.py
+++ test/lldbtest.py
@@ -32,6 +32,7 @@
"""
import abc
+import gc
import glob
import os, sys, traceback
import os.path
@@ -2544,6 +2545,11 @@
#import traceback
#traceback.print_stack()
+ # Ensure all the references to SB objects have gone away so that we can
+ # be sure that all the modules referenced by the target are orphaned and
+ # released.
+ 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
Index: source/Target/Target.cpp
===================================================================
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -1276,7 +1276,7 @@
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;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
