[issue25782] CPython hangs on error __context__ set to the error itself

Dong-hee Na added the comment: > Serhiy, I'm not closing this yet in case you'd like to finish implementing > your PR. If not, feel free to close. I check that PR 20543 makes CPython works correctly with msg399281 code. -- nosy: +corona10 ___

[issue25782] CPython hangs on error __context__ set to the error itself

Łukasz Langa added the comment: Serhiy, I'm not closing this yet in case you'd like to finish implementing your PR. If not, feel free to close. -- assignee: -> serhiy.storchaka ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

miss-islington added the comment: New changeset d86bbe3cff0abefc13e5462cca1fb3344d4a5b52 by Miss Islington (bot) in branch '3.10': bpo-25782: avoid hang in PyErr_SetObject when current exception has a cycle in its context chain (GH-27626)

[issue25782] CPython hangs on error __context__ set to the error itself

Łukasz Langa added the comment: New changeset 6f4cdeddb97532144f93ca37b8b21451f445c7bf by Miss Islington (bot) in branch '3.9': bpo-25782: avoid hang in PyErr_SetObject when current exception has a cycle in its context chain (GH-27626) (GH-27707)

[issue25782] CPython hangs on error __context__ set to the error itself

Change by miss-islington : -- pull_requests: +26192 pull_request: https://github.com/python/cpython/pull/27707 ___ Python tracker ___

[issue25782] CPython hangs on error __context__ set to the error itself

Change by miss-islington : -- nosy: +miss-islington nosy_count: 14.0 -> 15.0 pull_requests: +26191 pull_request: https://github.com/python/cpython/pull/27706 ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Łukasz Langa added the comment: New changeset d5c217475c4957a8084ac3f92ae012ece5edc7cb by Irit Katriel in branch 'main': bpo-25782: avoid hang in PyErr_SetObject when current exception has a cycle in its context chain (GH-27626)

[issue25782] CPython hangs on error __context__ set to the error itself

Nikita Sobolev added the comment: There's also a similar case with python3.9: ```python >>> class MyError(Exception): ... ... ... >>> e = MyError('e') >>> e.__context__ = e >>> >>> try: ... raise e ... except MyError: ... print('done') ... done # hangs after this ^C^Z ``` The same

[issue25782] CPython hangs on error __context__ set to the error itself

Irit Katriel added the comment: Note that my PR can (and should) be backported, while a change in the semantics of __context__ assignment, I'm not sure. -- ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Chris Jerdonek added the comment: > Preventing creation of the loop will fix all other code that iterate the > __context__ chain. We can still do / discuss that following Irit's proposed change, which is an improvement, IMO. -- ___ Python

[issue25782] CPython hangs on error __context__ set to the error itself

Irit Katriel added the comment: > My argument is the loop creation should be prevented at first place. I agree, but avoiding the hang is higher priority. > Preventing creation of the loop will fix all other code that iterate the > __context__ chain. There could still be a cycles involving

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: My argument is the loop creation should be prevented at first place. PyErr_SetObject() is not the only C code that can hang or overflow the stack when iterate a loop. Preventing creation of the loop will fix all other code that iterate the __context__

[issue25782] CPython hangs on error __context__ set to the error itself

Chris Jerdonek added the comment: > No, I meant C -> A -> B -> C -> A Oh, good. I support your reasoning and approach, by the way. -- ___ Python tracker ___

[issue25782] CPython hangs on error __context__ set to the error itself

Irit Katriel added the comment: > > the result is C -> A -> B -> C > Did you mean C -> A -> B? No, I meant C -> A -> B -> C -> A the cycle remains unchanged. > By the way, if you applied to this example your reasoning that > PyErr_SetObject shouldn't try to fix user bugs, should

[issue25782] CPython hangs on error __context__ set to the error itself

Chris Jerdonek added the comment: > the result is C -> A -> B -> C Did you mean C -> A -> B? By the way, if you applied to this example your reasoning that PyErr_SetObject shouldn't try to fix user bugs, should this example instead be C -> A -> B -> C -> ... (leaving the cycle as is but

[issue25782] CPython hangs on error __context__ set to the error itself

Irit Katriel added the comment: My patch doesn't change that at all, so it will be the same as it is currently: the result is C -> A -> B -> C. (But with my patch if you try to raise something in the context of C, it won't hang.) -- ___ Python

[issue25782] CPython hangs on error __context__ set to the error itself

Chris Jerdonek added the comment: That's okay. I didn't mean to suggest I thought your patch needed to handle that case or that we needed to decide it before moving forward. I was just wondering what would happen with your patch with it, even if it means a hang. Or were you saying that

[issue25782] CPython hangs on error __context__ set to the error itself

Irit Katriel added the comment: Serhiy's patch is modifying a different part of this system - he changes the Exception object's SetContext to break cycles when they are first created. Dennis and I targeted the place where an exception is about to be raised and it gets a __context__ that may

[issue25782] CPython hangs on error __context__ set to the error itself

Chris Jerdonek added the comment: Yes, that seems like a good approach. And how about with Serhiy's example from above? > If there is a chain A -> B -> C -> D -> E, after assignment C.__context__ = A > ... -- ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Change by Jack O'Connor : -- nosy: -oconnor663 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue25782] CPython hangs on error __context__ set to the error itself

Irit Katriel added the comment: Like Dennis' patch, mine changes PyErr_SetObject. The difference is that Dennis' patch gets rid of the cycle while mine leaves it as it is, just avoids hanging on it. So in this case: when trying to set the exception A on top of B -> C -> D -> E

[issue25782] CPython hangs on error __context__ set to the error itself

Chris Jerdonek added the comment: Thanks, Irit. Can you show how your patch behaves using some representative examples (maybe using arrow examples from above)? And if it behaves differently from Dennis's patch, can you include an example showing that, too? --

[issue25782] CPython hangs on error __context__ set to the error itself

Change by Larry Hastings : -- nosy: -larry ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue25782] CPython hangs on error __context__ set to the error itself

Irit Katriel added the comment: I added a third patch to the discussion. (It overlaps Dennis's suggestion, and I'm happy to close it in favour of a tweaked version of Dennis' patch, but I thought this would be the simplest way to say what I mean, and hopefully push the discussion along).

[issue25782] CPython hangs on error __context__ set to the error itself

Change by Irit Katriel : -- pull_requests: +26120 pull_request: https://github.com/python/cpython/pull/27626 ___ Python tracker ___

[issue25782] CPython hangs on error __context__ set to the error itself

Change by Irit Katriel : -- versions: +Python 3.11 -Python 3.6, Python 3.7, Python 3.8 ___ Python tracker ___ ___ Python-bugs-list

[issue25782] CPython hangs on error __context__ set to the error itself

Irit Katriel added the comment: I agree with Chris that the issue of not hanging is independent of the question what to do about the cycle. The ExitStack issue that brought this issue about is now fixed in ExitStack (see issue25786, issue27122). If we take a step back from that, the

[issue25782] CPython hangs on error __context__ set to the error itself

Change by STINNER Victor : -- nosy: -vstinner ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue25782] CPython hangs on error __context__ set to the error itself

Chris Jerdonek added the comment: I think this issue needs deeper discussion to know how to proceed. > If there is a chain A -> B -> C -> D -> E, after assignment C.__context__ = A > we will get a chain C -> A -> B -> D -> E. No exception is lost. I understand not wanting to lose exceptions

[issue25782] CPython hangs on error __context__ set to the error itself

Change by Chris Jerdonek : -- nosy: +chris.jerdonek ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue25782] CPython hangs on error __context__ set to the error itself

Change by Dennis Sweeney : -- pull_requests: +19788 pull_request: https://github.com/python/cpython/pull/20539 ___ Python tracker ___

[issue25782] CPython hangs on error __context__ set to the error itself

Change by Serhiy Storchaka : -- pull_requests: +19787 stage: -> patch review pull_request: https://github.com/python/cpython/pull/20543 ___ Python tracker ___

[issue25782] CPython hangs on error __context__ set to the error itself

Dennis Sweeney added the comment: For clarification, the existing behavior on master: When trying to raise the exception H, F -> G -> H -> I -> NULL becomes H -> F -> G -> NULL But when trying to set the exception A on top of B -> C -> D -> E -> C -> ...,

[issue25782] CPython hangs on error __context__ set to the error itself

Change by Gregory P. Smith : -- nosy: -gregory.p.smith ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: Issue40696 is other example of creating a cycle. I think we should solve general problem preventing loops by merging one of proposed patches. -- resolution: fixed -> stage: commit review -> status: closed -> open versions: +Python 3.10, Python

[issue25782] CPython hangs on error __context__ set to the error itself

Gregory P. Smith added the comment: I believe the original issue is fixed, the original problem and the one Jack followed up with no longer work in modern Python 3. But there is discussion about possibly doing something better, if anyone is interested in doing that I suggest

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: What should we do with this issue? Does it need opening a topic on Python-Dev? -- ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Changes by Guido van Rossum : -- nosy: -gvanrossum ___ Python tracker ___ ___

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: Because we have two alternate solutions, issue25782_5.patch and set_context_reordering2.patch. -- ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Larry Hastings added the comment: Why is this still open? GPS: didn't your checkin last June fix this? -- ___ Python tracker ___

[issue25782] CPython hangs on error __context__ set to the error itself

larsonreever added the comment: My patch works for your example too. Since it checks for loops in __context__ setter, you shouldn't be able to create complicated loops. However, since PyException_SetContext and PyException_SetCause are public APIs, and their return type is 'void', I can't

[issue25782] CPython hangs on error __context__ set to the error itself

Terry J. Reedy added the comment: Issue #28962 appears to be an example of this issue. It uses 'raise e from e' to create the loop. -- nosy: +terry.reedy ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: For reference: the correct issue number is #27122. Are there examples of creating a loop not involving ExitStack or explicit setting the __context__ attribute? -- ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Roundup Robot added the comment: New changeset 9ee36b74b432 by Gregory P. Smith in branch '3.5': Issue #27123: When an exception is raised within the context being https://hg.python.org/cpython/rev/9ee36b74b432 New changeset 9fadeee05880 by Gregory P. Smith in branch 'default': Issue #27123:

[issue25782] CPython hangs on error __context__ set to the error itself

Gregory P. Smith added the comment: I think I'm with Nick on this, the re-ordering fix is the closest to the right thing to do for 3.5 as it at least leaves all exceptions present in the chain. As a more accurate long term fix, it would be great if we could express the tree via tuples in

[issue25782] CPython hangs on error __context__ set to the error itself

Nick Coghlan added the comment: > But fixing this issue by reordering the exception chain will only mask bugs > that just better to be fixed. And sometimes, this will cause weird > exceptions chains, that will make *some* people spend a lot of time > debugging/googling to understand what's

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: > issue27122_broken_cm.py also shows why I think "make it work" is the right > answer here [..] But fixing this issue by reordering the exception chain will only mask bugs that just better to be fixed. And sometimes, this will cause weird exceptions chains,

[issue25782] CPython hangs on error __context__ set to the error itself

Nick Coghlan added the comment: Thanks Serhiy. I've attached a new file (issue27122_broken_cm.py) with a context manager that is deliberately buggy in the same way as contextlib._GeneratorContextManager is currently, so the new test can be made independent of #27122 being fixed (Greg Smith

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: With Yury's path the issue27122 test need to be changed (RuntimeError is now raised instead of original exception). Here is updated patch. -- Added file: http://bugs.python.org/file43381/Issue25782_5.patch ___

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: With reordering the issue27122 test is passed without changes. No changes in ExitStack is needed. Here is updated patch. -- Added file: http://bugs.python.org/file43380/set_context_reordering2.patch ___ Python

[issue25782] CPython hangs on error __context__ set to the error itself

Nick Coghlan added the comment: For both of the proposed patches, could we add a test case based on contextlib.ExitStack and a variant of Victor's #27122 reproducer script at http://bugs.python.org/file42999/hang_bug2.py that uses a deliberately broken __exit__ implementation that always

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: > Yury's raises a RuntimeError in the loop situation. > Serhiy's simply reorders the exception context to put the referred to one at > the front of the chain in the event of a loop. Right, and I believe that my solution is more Pythonic. Reordering feels

[issue25782] CPython hangs on error __context__ set to the error itself

Larry Hastings added the comment: I'm not the right person to decide this. As RM, all I can do is decide whether or not to hold up a release based on the bug. The answer: no. Since this isn't fixed yet in the 3.5 branch, 3.5.2 will go out without it being fixed. Sorry folks. --

[issue25782] CPython hangs on error __context__ set to the error itself

Changes by Gregory P. Smith : -- stage: commit review -> patch review ___ Python tracker ___

[issue25782] CPython hangs on error __context__ set to the error itself

Gregory P. Smith added the comment: Patch review: I see two competing solutions with differing behaviors. Yury's raises a RuntimeError in the loop situation. Serhiy's simply reorders the exception context to put the referred to one at the front of the chain in the event of a loop. The 3.5

[issue25782] CPython hangs on error __context__ set to the error itself

Changes by Rotem Yaari : -- nosy: +Rotem Yaari ___ Python tracker ___ ___ Python-bugs-list

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: And added more comments to your patch. -- ___ Python tracker ___ ___

[issue25782] CPython hangs on error __context__ set to the error itself

Changes by Serhiy Storchaka : Added file: http://bugs.python.org/file41354/set_context_reordering.patch ___ Python tracker ___

[issue25782] CPython hangs on error __context__ set to the error itself

Changes by Serhiy Storchaka : Removed file: http://bugs.python.org/file41353/set_context_reordering.patch ___ Python tracker ___

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: Here is a patch that illustrates my idea (the documentation could be better). -- Added file: http://bugs.python.org/file41353/set_context_reordering.patch ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: > Serhiy Storchaka added the comment: > > The patch LGTM (but I prefer reordering solution). Serhiy, could you please describe your solution in more detail? An elaborate example would help. Perhaps I just misunderstand your idea. And thanks for the review!

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: The patch LGTM (but I prefer reordering solution). -- stage: patch review -> commit review type: -> behavior versions: -Python 3.3, Python 3.4 ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: A new patch is attached. Please review. I decided to remove the fix for recursive __cause__. Currently, `raise e from e` doesn't cause any problem, and if we fix the interpreter to raise an RuntimeError in such cases it will be a backwards incompatible

[issue25782] CPython hangs on error __context__ set to the error itself

Larry Hastings added the comment: I'm not going to hold up 3.5.1 for this. -- priority: release blocker -> high ___ Python tracker ___

[issue25782] CPython hangs on error __context__ set to the error itself

Changes by Yury Selivanov : -- nosy: +georg.brandl, larry priority: normal -> release blocker ___ Python tracker ___

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: Looks like this is the original code committed in CPython in 2ee09afee126. Patch by Antoine Pitrou. Antoine, how would you fix this? -- nosy: +pitrou ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

New submission from Yury Selivanov: try: raise Exception except Exception as ex: ex.__context__ = ex hasattr(1, 'aa') -- components: Interpreter Core messages: 255731 nosy: gvanrossum, haypo, ncoghlan, yselivanov priority: normal severity: normal status: open title: CPython

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: I would change __context__ setter to check if it creates a loop. -- nosy: +serhiy.storchaka ___ Python tracker ___

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: The bug is in "PyErr_SetObject": while ((context = PyException_GetContext(o))) { Py_DECREF(context); if (context == value) { PyException_SetContext(o, NULL); break;

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: Serhiy, good idea, thanks! Please review the attached patch. Larry, I view this as a very serious bug. Can we ship 3.5.1 with it fixed? -- keywords: +patch Added file: http://bugs.python.org/file41212/Issue25782.patch

[issue25782] CPython hangs on error __context__ set to the error itself

Jack O'Connor added the comment: Yury, do we need to handle more complicated infinite loops, where "self" doesn't actually show up in the loop? Here's an example: try: raise Exception except Exception as ex: loop1 = Exception() loop2 = Exception()

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: >Don't do that, a few hours (!) is not enough to test a fix. It's too late after a RC1 for such critical change (exceptions). Maybe we can add an RC2? -- ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: > If there is a chain A -> B -> C -> D -> E, after assignment C.__context__ = A > we will get a chain C -> A -> B -> D -> E. No exception is lost. What to do when you try to chain "C -> C"? I'm not sure I like this reordering idea -- it might introduce some

[issue25782] CPython hangs on error __context__ set to the error itself

STINNER Victor added the comment: > Maybe we can add an RC2? Seriously? I'm waiting Python 3.5.1 since 3.5.0 was released. I'm amazed how much time it takes to release a first bugfix version, 3.5.0 was full a bugs (see the changelog). It's very easy to workaround this issue in pure Python.

[issue25782] CPython hangs on error __context__ set to the error itself

STINNER Victor added the comment: Yury Selivanov added the comment: > Please look at http://bugs.python.org/issue25779. I think we either should > fix this issue, or fix http://bugs.python.org/issue25786 in 3.5.1, since I > can't find a workaround for it. The latter issue is probably easier

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: > Should we do the same for __cause__? Is it possible to create __context__ or > __cause__ loop without assigning these attributes directly? Yes, let's mirror the __context__ behaviour for __cause__. New patch attached. Serhiy, Guido, The new patch raises

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: > Setting it to NULL is one option -- silently ignoring the assignment (leaving whatever was there) might also be good? In 90% of the cases it would be the same thing right? But leaving the old __context__ there will completely mask the bug... And as for pure

[issue25782] CPython hangs on error __context__ set to the error itself

Guido van Rossum added the comment: > But leaving the old __context__ there will completely mask the bug... OK, NULL is fine then. >we better raise a TypeError if a cycle is about to be introduced? Yes. -- ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: Yet one option is the emersion of the exception. Affected exception is removed from the chain and added at it head. If there is a chain A -> B -> C -> D -> E, after assignment C.__context__ = A we will get a chain C -> A -> B -> D -> E. No exception is

[issue25782] CPython hangs on error __context__ set to the error itself

STINNER Victor added the comment: > Larry, I view this as a very serious bug. Can we ship 3.5.1 with it fixed? Don't do that, a few hours (!) is not enough to test a fix. It's too late after a RC1 for such critical change (exceptions). The bug was here since at least Python 3.3, there is no

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: > Yury, do we need to handle more complicated infinite loops, where "self" > doesn't actually show up in the loop? Here's an example: My patch works for your example too. Since it checks for loops in __context__ setter, you shouldn't be able to create

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: > It's very easy to workaround this issue in pure Python. Why do you want the > fix *RIGHT NOW*? Please look at http://bugs.python.org/issue25779. I think we either should fix this issue, or fix http://bugs.python.org/issue25786 in 3.5.1, since I can't find

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: Should we do the same for __cause__? Is it possible to create __context__ or __cause__ loop without assigning these attributes directly? -- ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Guido van Rossum added the comment: Ouch, it's unfortunate those APIs don't have an error return. :-( Setting it to NULL is one option -- silently ignoring the assignment (leaving whatever was there) might also be good? In 90% of the cases it would be the same thing right? (I'm not familiar

[issue25782] CPython hangs on error __context__ set to the error itself

Serhiy Storchaka added the comment: > What to do when you try to chain "C -> C"? Nothing. Or may be raise an exception (because C.__context__ can't be set to what you try). > you expected one type of exception, then you used some external library, and > after that you have a completely

[issue25782] CPython hangs on error __context__ set to the error itself

Larry Hastings added the comment: > Please look at http://bugs.python.org/issue25779. You closed that one and marked it "not a bug"...? -- ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: > You closed that one and marked it "not a bug"...? That particular issue was about asyncio. After reducing it, I created two new issues -- this one, and another one about another bug in contextlib. -- ___ Python

[issue25782] CPython hangs on error __context__ set to the error itself

Yury Selivanov added the comment: Serhiy, Victor, thank you for your reviews. Another version of the patch is attached. -- Added file: http://bugs.python.org/file41219/Issue25782_3.patch ___ Python tracker

[issue25782] CPython hangs on error __context__ set to the error itself

Changes by Antoine Pitrou : -- nosy: -pitrou ___ Python tracker ___ ___ Python-bugs-list