It is possible to create a loop by setting the __context__ attribute of the raised exception, either explicitly, or implicitly, using "raise ... from ...". Creating a loop can leads to the hanging in infinite loop or crash due to stack overflow. Several years ago we did have issues related to ExitStack and contextmanager() [1] [2], and recently we have found other issue related to asynccontextmanager() [3]. There may be other issues in the stdlib or third-party libraries.
To solve all such issues we can prevent creating the loop in the __context__ setter. Unfortunately there is no way to return an error in PyException_SetContext(), so it is always success. So we should break a loop at some point. There was proposed several options. If we have a chain
A -> B -> C -> D -> E -> NULL
after raising C in the context of A ("raise C from A") or setting C.__context__ = A we will get a chain:
- C -> A -> B -> C -> A -> B -> ...
A loop. This is the current behavior.
- C -> D -> E -> NULL
The operation is no-op. We lose information about exceptions preceding C: A and B.
- C -> NULL
Lose all context.
- C -> A -> B -> D -> E -> NULL
C is moved at the front of the chain. No exception is lost, but their order is changed.
- C -> A -> B -> NULL
Remove C from the chain and rewrite its context. We lose information about the old context of C: D and E.
- C -> A -> B -> C' -> D -> E -> NULL
Create a copy of C (how?) and replace C with its copy in the chain. But what to do with other references to C?
There may be other options.
We need to solve this issue, otherwise we will continue to stick with other bugs related to the exceptions loop.
[1] https://bugs.python.org/issue25779 [2] https://bugs.python.org/issue25786 [3] https://bugs.python.org/issue40696 [4] https://bugs.python.org/issue25782
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.
On Sun, Jun 14, 2020 at 9:19 AM Serhiy Storchaka storchaka@gmail.com wrote:
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 sweeney.dennis650@gmail.com wrote:
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/
-
Chris Jerdonek -
Dennis Sweeney -
Serhiy Storchaka