Make exception handling thread-safe by using sys.exc_info() instead of sys.exc_{type,value}.

9 files changed, 102 insertions, 103 deletions

diff --git a/src/engine/SCons/Errors.py b/src/engine/SCons/Errors.py
index 283e681..7eb3098 100644
--- a/src/engine/SCons/Errors.py
+++ b/src/engine/SCons/Errors.py
@@ -58,10 +58,3 @@ class ConfigureDryRunError(UserError):
     but the user requested a dry-run"""
     def __init__(self,file):
         UserError.__init__(self,"Cannot update configure test (%s) within a dry-run." % str(file))
-
-class TaskmasterException(Exception):
-    def __init__(self, type, value, traceback, *args):
-        self.type = type
-        self.value = value
-        self.traceback = traceback
-        apply(Exception.__init__, (self,) + args)
diff --git a/src/engine/SCons/ErrorsTests.py b/src/engine/SCons/ErrorsTests.py
index bb47610..248b366 100644
--- a/src/engine/SCons/ErrorsTests.py
+++ b/src/engine/SCons/ErrorsTests.py
@@ -65,15 +65,6 @@ class ErrorsTestCase(unittest.TestCase):
         except SCons.Errors.UserError, e:
             assert e.args == ("Cannot update configure test (FileName) within a dry-run.",)
 
-    def test_TaskmasterException(self):
-        """Test the TaskmasterException."""
-        try:
-            raise SCons.Errors.TaskmasterException("one", "two", "three")
-        except SCons.Errors.TaskmasterException, e:
-            assert e.type == "one", e.type
-            assert e.value == "two", e.value
-            assert e.traceback == "three", e.traceback
-
 
 if __name__ == "__main__":
     suite = unittest.makeSuite(ErrorsTestCase, 'test_')
diff --git a/src/engine/SCons/Job.py b/src/engine/SCons/Job.py
index e5f9a71..bbe0a2d 100644
--- a/src/engine/SCons/Job.py
+++ b/src/engine/SCons/Job.py
@@ -112,6 +112,7 @@ class Serial:
             except KeyboardInterrupt:
                 raise
             except:
+                task.exception_set()
                 # Let the failed() callback function arrange for the
                 # build to stop if that's appropriate.
                 task.failed()
@@ -152,6 +153,7 @@ else:
                     # be explicit here for test/interrupts.py
                     ok = False
                 except:
+                    task.exception_set()
                     ok = 0
                 else:
                     ok = 1
diff --git a/src/engine/SCons/JobTests.py b/src/engine/SCons/JobTests.py
index 48caa16..d216464 100644
--- a/src/engine/SCons/JobTests.py
+++ b/src/engine/SCons/JobTests.py
@@ -136,6 +136,9 @@ class ExceptionTask:
     def postprocess(self):
         self.taskmaster.num_postprocessed = self.taskmaster.num_postprocessed + 1
 
+    def exception_set(self):
+        self.taskmaster.exception_set()
+
 class Taskmaster:
     """A dummy taskmaster class for testing the job classes."""
 
@@ -197,6 +200,9 @@ class Taskmaster:
                                  == (i + 1))
         return serial
 
+    def exception_set(self):
+        pass
+
 class ParallelTestCase(unittest.TestCase):
     def runTest(self):
         "test parallel jobs"
diff --git a/src/engine/SCons/SConf.py b/src/engine/SCons/SConf.py
index 6280f75..0cda8c1 100644
--- a/src/engine/SCons/SConf.py
+++ b/src/engine/SCons/SConf.py
@@ -189,13 +189,16 @@ class SConf:
                 if (target.get_state() != SCons.Node.up_to_date and
                     target.has_builder() and
                     not hasattr(target.builder, 'status')):
-                    
+
                     raise SCons.Errors.ConfigureDryRunError(target)
                 
             def failed(self):
-                if sys.exc_type == SCons.Errors.ConfigureDryRunError:
-                    raise
-                SConfBuildTask.failed(self)
+                exc_type, exc_value = self.exc_info()[:2]
+                if exc_type == SCons.Errors.ConfigureDryRunError:
+                    raise exc_type, exc_value
+                # Should be SConfBuildTask.failed(), really,
+                # but that causes name errors in Python 1.5.2.
+                SCons.Script.BuildTask.failed(self)
 
         if self.logstream != None:
             # override stdout / stderr to write in log file
diff --git a/src/engine/SCons/Script/SConscript.py b/src/engine/SCons/Script/SConscript.py
index 81450e9..074477f 100644
--- a/src/engine/SCons/Script/SConscript.py
+++ b/src/engine/SCons/Script/SConscript.py
@@ -270,7 +270,8 @@ def SConscript_exception(file=sys.stderr):
     This will show users who have Python errors where the problem is,
     without cluttering the output with all of the internal calls leading
     up to where we exec the SConscript."""
-    stack = traceback.extract_tb(sys.exc_traceback)
+    exc_type, exc_value, exc_tb = sys.exc_info()
+    stack = traceback.extract_tb(exc_tb)
     last_text = ""
     found = 0
     i = 0
@@ -284,10 +285,10 @@ def SConscript_exception(file=sys.stderr):
         # We did not find our exec statement, so this was actually a bug
         # in SCons itself.  Show the whole stack.
         i = 0
-    type = str(sys.exc_type)
+    type = str(exc_type)
     if type[:11] == "exceptions.":
         type = type[11:]
-    file.write('%s: %s:\n' % (type, sys.exc_value))
+    file.write('%s: %s:\n' % (type, exc_value))
     for fname, line, func, text in stack[i:]:
         file.write('  File "%s", line %d:\n' % (fname, line))
         file.write('    %s\n' % text)
diff --git a/src/engine/SCons/Script/__init__.py b/src/engine/SCons/Script/__init__.py
index 2dbb961..87e9602 100644
--- a/src/engine/SCons/Script/__init__.py
+++ b/src/engine/SCons/Script/__init__.py
@@ -153,18 +153,12 @@ class BuildTask(SCons.Taskmaster.Task):
         # is to display the various types of Errors and Exceptions
         # appropriately.
         status = 2
-        e = sys.exc_value
-        t = sys.exc_type
+        t, e = self.exc_info()[:2]
         tb = None
-        if t is SCons.Errors.TaskmasterException:
-            # The Taskmaster received an Error or Exception while trying
-            # to process or build the Nodes and dependencies, which it
-            # wrapped up for us in the object recorded as the value of
-            # the Exception, so process the wrapped info instead of the
-            # TaskmasterException itself.
-            t = e.type
-            tb = e.traceback
-            e = e.value
+        if t is None:
+            # The Taskmaster didn't record an exception for this Task;
+            # see if the sys module has one.
+            t, e = sys.exc_info()[:2]
 
         if t == SCons.Errors.BuildError:
             sys.stderr.write("scons: *** [%s] %s\n" % (e.node, e.errstr))
@@ -187,6 +181,8 @@ class BuildTask(SCons.Taskmaster.Task):
 
         self.do_failed(status)
 
+        self.exc_clear()
+
     def make_ready(self):
         """Make a task ready for execution"""
         SCons.Taskmaster.Task.make_ready(self)
diff --git a/src/engine/SCons/Taskmaster.py b/src/engine/SCons/Taskmaster.py
index 7fb94aa..5bfa3d6 100644
--- a/src/engine/SCons/Taskmaster.py
+++ b/src/engine/SCons/Taskmaster.py
@@ -102,17 +102,19 @@ class Task:
         except KeyboardInterrupt:
             raise
         except SystemExit:
-            raise SCons.Errors.ExplicitExit(self.targets[0], sys.exc_value.code)
+            exc_value = sys.exc_info()[1]
+            raise SCons.Errors.ExplicitExit(self.targets[0], exc_value.code)
         except SCons.Errors.UserError:
             raise
         except SCons.Errors.BuildError:
             raise
         except:
+            exc_type, exc_value, exc_traceback = sys.exc_info()
             raise SCons.Errors.BuildError(self.targets[0],
                                           "Exception",
-                                          sys.exc_type,
-                                          sys.exc_value,
-                                          sys.exc_traceback)
+                                          exc_type,
+                                          exc_value,
+                                          exc_traceback)
 
     def get_target(self):
         """Fetch the target being built or updated by this task.
@@ -206,6 +208,15 @@ class Task:
         for t in self.targets:
             t.postprocess()
 
+    def exc_info(self):
+        return self.tm.exception
+
+    def exc_clear(self):
+        self.tm.exception_clear()
+
+    def exception_set(self):
+        self.tm.exception_set()
+
 
 
 def order(dependencies):
@@ -229,7 +240,7 @@ class Taskmaster:
         self.tasker = tasker
         self.ready = None # the next task that is ready to be executed
         self.order = order
-        self.exception_set(None, None)
+        self.exception_clear()
         self.message = None
 
     def _find_next_ready_node(self):
@@ -253,8 +264,9 @@ class Taskmaster:
             try:
                 children = node.children()
             except SystemExit:
-                e = SCons.Errors.ExplicitExit(node, sys.exc_value.code)
-                self.exception_set(SCons.Errors.ExplicitExit, e)
+                exc_value = sys.exc_info()[1]
+                e = SCons.Errors.ExplicitExit(node, exc_value.code)
+                self.exception_set((SCons.Errors.ExplicitExit, e))
                 self.candidates.pop()
                 self.ready = node
                 break
@@ -265,10 +277,7 @@ class Taskmaster:
                 # children (like a child couldn't be linked in to a
                 # BuildDir, or a Scanner threw something).  Arrange to
                 # raise the exception when the Task is "executed."
-                x = SCons.Errors.TaskmasterException(sys.exc_type,
-                                                     sys.exc_value,
-                                                     sys.exc_traceback)
-                self.exception_set(x)
+                self.exception_set()
                 self.candidates.pop()
                 self.ready = node
                 break
@@ -294,10 +303,7 @@ class Taskmaster:
                 # the kids are derived (like a child couldn't be linked
                 # from a repository).  Arrange to raise the exception
                 # when the Task is "executed."
-                x = SCons.Errors.TaskmasterException(sys.exc_type,
-                                                     sys.exc_value,
-                                                     sys.exc_traceback)
-                self.exception_set(x)
+                self.exception_set()
                 self.candidates.pop()
                 self.ready = node
                 break
@@ -381,10 +387,7 @@ class Taskmaster:
             # a child couldn't be linked in to a BuildDir when deciding
             # whether this node is current).  Arrange to raise the
             # exception when the Task is "executed."
-            x = SCons.Errors.TaskmasterException(sys.exc_type,
-                                                 sys.exc_value,
-                                                 sys.exc_traceback)
-            self.exception_set(x)
+            self.exception_set()
         self.ready = None
 
         return task
@@ -429,23 +432,21 @@ class Taskmaster:
         self.candidates.extend(self.pending)
         self.pending = []
 
-    def exception_set(self, type, value=None):
-        """Record an exception type and value to raise later, at an
-        appropriate time."""
-        self.exc_type = type
-        self.exc_value = value
-        self.exc_traceback = traceback
+    def exception_set(self, exception=None):
+        if exception is None:
+            exception = sys.exc_info()
+        self.exception = exception
+        self.exception_raise = self._exception_raise
+
+    def exception_clear(self):
+        self.exception = (None, None, None)
+        self.exception_raise = self._no_exception_to_raise
 
-    def exception_raise(self):
-        """Raise any pending exception that was recorded while
+    def _no_exception_to_raise(self):
+        pass
+
+    def _exception_raise(self):
+        """Raise a pending exception that was recorded while
         getting a Task ready for execution."""
-        if self.exc_type:
-            try:
-                try:
-                    raise self.exc_type, self.exc_value
-                except TypeError:
-                    # exc_type was probably an instance,
-                    # so raise it by itself.
-                    raise self.exc_type
-            finally:
-                self.exception_set(None, None)
+        exc_type, exc_value = self.exception[:2]
+        raise exc_type, exc_value
diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py
index ebf0e9c..040d52a 100644
--- a/src/engine/SCons/TaskmasterTests.py
+++ b/src/engine/SCons/TaskmasterTests.py
@@ -472,11 +472,9 @@ class TaskmasterTestCase(unittest.TestCase):
         n1 = Node("n1")
         tm = SCons.Taskmaster.Taskmaster(targets = [n1], tasker = MyTask)
         t = tm.next_task()
-        assert isinstance(tm.exc_type, SCons.Errors.TaskmasterException), repr(tm.exc_type)
-        assert tm.exc_value is None, tm.exc_value
-        e = tm.exc_type
-        assert e.type == MyException, e.type
-        assert str(e.value) == "from make_ready()", str(e.value)
+        exc_type, exc_value, exc_tb = tm.exception
+        assert exc_type == MyException, repr(exc_type)
+        assert str(exc_value) == "from make_ready()", exc_value
 
 
     def test_make_ready_all(self):
@@ -553,18 +551,17 @@ class TaskmasterTestCase(unittest.TestCase):
         n1 = StopNode("n1")
         tm = SCons.Taskmaster.Taskmaster([n1])
         t = tm.next_task()
-        assert isinstance(tm.exc_type, SCons.Errors.TaskmasterException), repr(tm.exc_type)
-        assert tm.exc_value is None, tm.exc_value
-        e = tm.exc_type
-        assert e.type == SCons.Errors.StopError, e.type
-        assert str(e.value) == "stop!", "Unexpected exc_value `%s'" % e.value
+        exc_type, exc_value, exc_tb = tm.exception
+        assert exc_type == SCons.Errors.StopError, repr(exc_type)
+        assert str(exc_value) == "stop!", exc_value
 
         n2 = ExitNode("n2")
         tm = SCons.Taskmaster.Taskmaster([n2])
         t = tm.next_task()
-        assert tm.exc_type == SCons.Errors.ExplicitExit, "Did not record ExplicitExit on node"
-        assert tm.exc_value.node == n2, tm.exc_value.node
-        assert tm.exc_value.status == 77, tm.exc_value.status
+        exc_type, exc_value = tm.exception
+        assert exc_type == SCons.Errors.ExplicitExit, repr(exc_type)
+        assert exc_value.node == n2, exc_value.node
+        assert exc_value.status == 77, exc_value.status
 
     def test_cycle_detection(self):
         """Test detecting dependency cycles
@@ -738,8 +735,7 @@ class TaskmasterTestCase(unittest.TestCase):
         built_text = None
         n5 = Node("n5")
         tm = SCons.Taskmaster.Taskmaster([n5])
-        tm.exc_type = MyException
-        tm.exc_value = "exception value"
+        tm.exception_set((MyException, "exception value"))
         t = tm.next_task()
         exc_caught = None
         try:
@@ -830,7 +826,8 @@ class TaskmasterTestCase(unittest.TestCase):
             assert len(e.args) == 3, `e.args`
             assert e.args[0] == OtherError, e.args[0]
             assert isinstance(e.args[1], OtherError), type(e.args[1])
-            assert type(e.args[2]) == type(sys.exc_traceback), e.args[2]
+            exc_traceback = sys.exc_info()[2]
+            assert type(e.args[2]) == type(exc_traceback), e.args[2]
         else:
             raise TestFailed, "did not catch expected BuildError"
 
@@ -878,33 +875,42 @@ class TaskmasterTestCase(unittest.TestCase):
         n1 = Node("n1")
         tm = SCons.Taskmaster.Taskmaster([n1])
 
-        tm.exception_set(1, 2)
-        assert tm.exc_type == 1, tm.exc_type
-        assert tm.exc_value == 2, tm.exc_value
+        tm.exception_set((1, 2))
+        exc_type, exc_value = tm.exception
+        assert exc_type == 1, exc_type
+        assert exc_value == 2, exc_value
 
         tm.exception_set(3)
-        assert tm.exc_type == 3, tm.exc_type
-        assert tm.exc_value is None, tm.exc_value
-
-        tm.exception_set(None, None)
-        assert tm.exc_type is None, tm.exc_type
-        assert tm.exc_value is None, tm.exc_value
-
-        tm.exception_set("exception 1", None)
+        assert tm.exception == 3
+
+        try: 1/0
+        except: pass
+        tm.exception_set(None)
+        exc_type, exc_value, exc_tb = tm.exception
+        assert exc_type is ZeroDivisionError, exc_type
+        exception_values = [
+            "integer division or modulo",
+            "integer division or modulo by zero",
+        ]
+        assert str(exc_value) in exception_values, exc_value
+
+        tm.exception_set(("exception 1", None))
         try:
             tm.exception_raise()
         except:
-            assert sys.exc_type == "exception 1", sys.exc_type
-            assert sys.exc_value is None, sys.exc_value
+            exc_type, exc_value = sys.exc_info()[:2]
+            assert exc_type == "exception 1", exc_type
+            assert exc_value is None, exc_value
         else:
             assert 0, "did not catch expected exception"
 
-        tm.exception_set("exception 2", "xyzzy")
+        tm.exception_set(("exception 2", "xyzzy"))
         try:
             tm.exception_raise()
         except:
-            assert sys.exc_type == "exception 2", sys.exc_type
-            assert sys.exc_value == "xyzzy", sys.exc_value
+            exc_type, exc_value = sys.exc_info()[:2]
+            assert exc_type == "exception 2", exc_type
+            assert exc_value == "xyzzy", exc_value
         else:
             assert 0, "did not catch expected exception"