[pypy-commit] pypy py3k: issue1903: remove the lazy __context__ recording potentially getting the wrong

pjenvey pypy.commits at gmail.com
Tue Apr 26 19:08:58 EDT 2016


Author: Philip Jenvey <pjenvey at underboss.org>
Branch: py3k
Changeset: r83931:baabf9008933
Date: 2016-04-26 16:07 -0700
http://bitbucket.org/pypy/pypy/changeset/baabf9008933/

Log:	issue1903: remove the lazy __context__ recording potentially getting
	the wrong result

	we still defer the recording -- until OperationErrors raise their
	way back through the interpreter (instead of @ their instantiation).
	this + unrolling the stack walking doesn't seem to hurt us on the
	runnable bits of the Grand Unified Python benchmark suite

diff --git a/pypy/interpreter/error.py b/pypy/interpreter/error.py
--- a/pypy/interpreter/error.py
+++ b/pypy/interpreter/error.py
@@ -31,6 +31,7 @@
 
     _w_value = None
     _application_traceback = None
+    _context_recorded = False
     w_cause = None
 
     def __init__(self, w_type, w_value, tb=None, w_cause=None):
@@ -329,52 +330,37 @@
         self._application_traceback = tb
 
     def record_context(self, space, frame):
-        """Record a __context__ for this exception from the current
-        frame if one exists.
+        """Record a __context__ for this exception if one exists,
+        searching from the current frame.
+        """
+        if self._context_recorded:
+            return
+        last = frame._exc_info_unroll(space)
+        try:
+            if last is not None:
+                self.normalize_exception(space)
+                w_value = self.get_w_value(space)
+                w_last = last.get_w_value(space)
+                if not space.is_w(w_value, w_last):
+                    _break_context_cycle(space, w_value, w_last)
+                    space.setattr(w_value, space.wrap('__context__'), w_last)
+        finally:
+            self._context_recorded = True
 
-        __context__ is otherwise lazily determined from the
-        traceback. However the current frame.last_exception must be
-        checked for a __context__ before this OperationError overwrites
-        it (making the previous last_exception unavailable later on).
-        """
-        last_exception = frame.last_exception
-        if (last_exception is not None and not frame.hide() or
-            last_exception is get_cleared_operation_error(space)):
-            # normalize w_value so setup_context can check for cycles
-            self.normalize_exception(space)
-            w_value = self.get_w_value(space)
-            w_last = last_exception.get_w_value(space)
-            if not space.is_w(w_value, w_last):
-                w_context = setup_context(space, w_value, w_last, lazy=True)
-                space.setattr(w_value, space.wrap('__context__'), w_context)
 
+def _break_context_cycle(space, w_value, w_context):
+    """Break reference cycles in the __context__ chain.
 
-def setup_context(space, w_exc, w_last, lazy=False):
-    """Determine the __context__ for w_exc from w_last and break
-    reference cycles in the __context__ chain.
+    This is O(chain length) but context chains are usually very short
     """
-    from pypy.module.exceptions.interp_exceptions import W_BaseException
-    if space.is_w(w_exc, w_last):
-        w_last = space.w_None
-    # w_last may also be space.w_None if from ClearedOpErr
-    if not space.is_w(w_last, space.w_None):
-        # Avoid reference cycles through the context chain. This is
-        # O(chain length) but context chains are usually very short.
-        w_obj = w_last
-        while True:
-            assert isinstance(w_obj, W_BaseException)
-            if lazy:
-                w_context = w_obj.w_context
-            else:
-                # triggers W_BaseException._setup_context
-                w_context = space.getattr(w_obj, space.wrap('__context__'))
-            if space.is_none(w_context):
-                break
-            if space.is_w(w_context, w_exc):
-                w_obj.w_context = space.w_None
-                break
-            w_obj = w_context
-    return w_last
+    while True:
+        w_next = space.getattr(w_context, space.wrap('__context__'))
+        if space.is_w(w_next, space.w_None):
+            break
+        if space.is_w(w_next, w_value):
+            space.setattr(w_context, space.wrap('__context__'), space.w_None)
+            break
+        w_context = w_next
 
 
 class ClearedOpErr:
diff --git a/pypy/interpreter/pyopcode.py b/pypy/interpreter/pyopcode.py
--- a/pypy/interpreter/pyopcode.py
+++ b/pypy/interpreter/pyopcode.py
@@ -73,6 +73,7 @@
         try:
             next_instr = self.dispatch_bytecode(co_code, next_instr, ec)
         except OperationError, operr:
+            operr.record_context(self.space, self)
             next_instr = self.handle_operation_error(ec, operr)
         except RaiseWithExplicitTraceback, e:
             next_instr = self.handle_operation_error(ec, e.operr,
diff --git a/pypy/interpreter/pytraceback.py b/pypy/interpreter/pytraceback.py
--- a/pypy/interpreter/pytraceback.py
+++ b/pypy/interpreter/pytraceback.py
@@ -61,7 +61,6 @@
     tb = operror.get_traceback()
     tb = PyTraceback(space, frame, last_instruction, tb)
     operror.set_traceback(tb)
-    operror.record_context(space, frame)
 
 
 def check_traceback(space, w_tb, msg):
diff --git a/pypy/interpreter/test/test_raise.py b/pypy/interpreter/test/test_raise.py
--- a/pypy/interpreter/test/test_raise.py
+++ b/pypy/interpreter/test/test_raise.py
@@ -387,8 +387,6 @@
         except:
             func1()
 
-    @py.test.mark.xfail(reason="A somewhat contrived case that may burden the "
-                        "JIT to fully support")
     def test_frame_spanning_cycle_broken(self):
         context = IndexError()
         def func():
@@ -399,7 +397,6 @@
                     raise context
                 except Exception as e2:
                     assert e2.__context__ is e1
-                    # XXX:
                     assert e1.__context__ is None
             else:
                 fail('No exception raised')
@@ -419,6 +416,7 @@
             except ValueError as exc:
                 assert exc.__cause__ is None
                 assert exc.__suppress_context__ is True
+                assert isinstance(exc.__context__, TypeError)
                 exc.__suppress_context__ = False
                 raise exc
         except ValueError as exc:
@@ -428,6 +426,19 @@
         assert isinstance(e.__context__, TypeError)
         """
 
+    def test_context_in_builtin(self):
+        context = IndexError()
+        try:
+            try:
+                raise context
+            except:
+                compile('pass', 'foo', 'doh')
+        except ValueError as e:
+            assert e.__context__ is context
+        else:
+            fail('No exception raised')
+
+
 class AppTestTraceback:
 
     def test_raise_with___traceback__(self):
diff --git a/pypy/module/exceptions/interp_exceptions.py b/pypy/module/exceptions/interp_exceptions.py
--- a/pypy/module/exceptions/interp_exceptions.py
+++ b/pypy/module/exceptions/interp_exceptions.py
@@ -92,7 +92,7 @@
     TypeDef, GetSetProperty, interp_attrproperty,
     descr_get_dict, descr_set_dict, descr_del_dict)
 from pypy.interpreter.gateway import interp2app, unwrap_spec
-from pypy.interpreter.error import OperationError, oefmt, setup_context
+from pypy.interpreter.error import OperationError, oefmt
 from pypy.interpreter.pytraceback import PyTraceback, check_traceback
 from rpython.rlib import rwin32
 
@@ -179,28 +179,7 @@
         self.suppress_context = True
 
     def descr_getcontext(self, space):
-        w_context = self.w_context
-        if w_context is None:
-            self.w_context = w_context = self._setup_context(space)
-        return w_context
-
-    def _setup_context(self, space):
-        """Lazily determine __context__ from w_traceback"""
-        # XXX: w_traceback can be overwritten: it's not necessarily the
-        # authoratative traceback!
-        last_operr = None
-        w_traceback = self.w_traceback
-        if w_traceback is not None and isinstance(w_traceback, PyTraceback):
-            # search for __context__ beginning in the previous frame. A
-            # __context__ from the top most frame would have already
-            # been handled by OperationError.record_context
-            frame = w_traceback.frame.f_backref()
-            if frame:
-                last_operr = frame._exc_info_unroll(space)
-        if last_operr is None:
-            # no __context__
-            return space.w_None
-        return setup_context(space, self, last_operr.get_w_value(space))
+        return self.w_context
 
     def descr_setcontext(self, space, w_newcontext):
         if not (space.is_w(w_newcontext, space.w_None) or


More information about the pypy-commit mailing list