On Sun, Jun 14, 2020 at 9:19 AM Serhiy Storchaka <storch...@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.dennis...@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-le...@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/R5J5JVUJX3V4DBKVLUI2SUBRD3TRF6PV/ > Code of Conduct: http://python.org/psf/codeofconduct/ >
_______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@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/OBCKNBRN4YA5DTCWEXUX5EIRUWBSQFCA/ Code of Conduct: http://python.org/psf/codeofconduct/
