On Sun, Jun 14, 2020 at 9:19 AM Serhiy Storchaka
It is possible to create a loop by setting the __context__ attribute of the raised exception, either explicitly, or implicitly, using "raise ... from ...".
I think we should separate the questions of what to do when (1) setting the
context explicitly (e.g. setting the __context__ attribute) and (2) setting
it implicitly (using e.g. the raise syntax).
When setting it explicitly, I think it's acceptable to lose the previous
context (because it's being done explicitly). But when done implicitly,
there's an argument that we should make an effort to preserve the previous
context somewhere, so information isn't lost.
I also think we should be open to the option of allowing cycles to exist,
and not artificially breaking them. There are a few reasons for this.
First, cycles in the chain can already exist in the current code. I believe
Python's traceback-formatting code already has logic to look for cycles, so
it won't cause hangs there. The reason for the most recent hang was
different: _PyErr_SetObject() has separate logic to look for cycles, and
that cycle-detection logic has a bug that can cause hangs. Finally, if we
preserve cycles, we can improve Python's traceback-formatting code to
display that the exception chain has a cycle. This is ideal IMO because the
user will learn of the issue and we won't destroy information.
I think it's important that we aim for a solution where the user is able to
learn if they have an issue with their exception-handling code (e.g. code
that creates cycles). This means we shouldn't silently try to alter or
"fix" the chain. Rather, we should display that there is a cycle (e.g. in
the traceback-formatting code). A couple more options would be to (1) issue
a warning if we are doing any artificial cycle breaking, and (2) insert a
new placeholder exception where the cycle starts, with the exception string
containing information about the cycle that was broken, so the user learns
about it and can fix it.
--Chris
On Mon, Jun 15, 2020 at 12:42 AM Dennis Sweeney
Worth noting is that there is an existing loop-breaking mechanism, but only for the newest exception being raised. In particular, option (4) is actually the current behavior if the the most recent exception participates in a cycle:
Python 3.9.0b1 >>> A, B, C, D, E = map(Exception, "ABCDE") >>> A.__context__ = B >>> B.__context__ = C >>> C.__context__ = D >>> D.__context__ = E >>> try: ... raise A ... except Exception: ... raise C ... Exception: B
During handling of the above exception, another exception occurred:
Traceback (most recent call last): File "<stdin>", line 2, in <module> Exception: A
During handling of the above exception, another exception occurred:
Traceback (most recent call last): File "<stdin>", line 4, in <module> Exception: C
This cycle-breaking is not due to any magic in the ``PyException_SetContext()``, which is currently a basic one-liner, but instead comes from ``_PyErr_SetObject`` in errors.c, which has something along the lines of:
def _PyErr_SetObject(new_exc): top = existing_topmost_exc()
if top is None: # no context set_top_exception(new_exc) return
# convert new_exc class to instance if applicable. ...
if top is new_exc: # already on top return
e = top while True: context = e.__context__ if context is None: # no loop break if context is new_exc: # unlink the existing exception e.__context__ = None break e = context
new_exc.__context__ = top set_top_exception(new_exc)
The only trouble comes about when there is a "rho-shaped" linked list, in which we have a cycle not involving the new exception being raised. For instance,
Raising A on top of (B -> C -> D -> C -> D -> C -> ...) results in an infinite loop.
Two possible fixes would be to either (I) use a magical ``__context__`` setter to ensure that there is never a rho-shaped sequence, or (II) allow arbitrary ``__context__`` graphs and then correctly handle rho-shaped sequences in ``_PyErr_SetObject`` (i.e. at raise-time).
Fix type (I) could result in surprising things like:
>>> A = Exception() >>> A.__context__ = A >>> A.__context__ is None True
so I propose fix type (II). This PR is such a fix: https://github.com/python/cpython/pull/20539
It basically extends the existing behavior (4) to the rho-shaped case.
It also prevents the cycle-detecting logic from sitting in two places (both _PyErr_SetObject and PyException_SetContext) and does not make any visible functionality more magical. The only Python-visible change should be that the infinite loop is no longer possible. _______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-leave@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/R5J5JVUJ... Code of Conduct: http://python.org/psf/codeofconduct/