bpo-25782: avoid hang in PyErr_SetObject when current exception has a…

[iritkatriel]

… cycle in its context chain

Co-authored-by: Dennis Sweeney [email protected]

https://bugs.python.org/issue25782

[sweeneyde]

Maybe it would be nice to have a test case with a longer cycle? As in

    def test_no_hang_on_context_chain_cycle2(self):
        class A(Exception):
            pass
        class B(Exception):
            pass
        class C(Exception):
            pass
        class D(Exception):
            pass
        class E(Exception):
            pass
        
        # Context cycle:
        #             +-----------+
        #             V           |
        # E --> D --> C --> B --> A
        with self.assertRaises(E):
            try:
                raise A()
            except A as _a:
                a = _a
                try:
                    raise B()
                except B as _b:
                    b = _b
                    try:
                        raise C()
                    except C as _c:
                        c = _c
                        a.__context__ = c
                        try:
                            raise D()
                        except D as _d:
                            d = _d
                            e = E()
                            raise e

        # Finish without breaking the cycle
        self.assertIs(e.__context__, d)
        self.assertIs(d.__context__, c)
        self.assertIs(c.__context__, b)
        self.assertIs(b.__context__, a)
        self.assertIs(a.__context__, c)

Also, would it be good to check that the traceback doesn't get stuck in a loop when printing? I tried it, and it terminates as you'd hope, but it's unclear to me why that's the case.

[iritkatriel]

The traceback printing code does its own cycle detection. I'll extend the test, thanks.

[cjerdonek]

If you do add a longer test, I would keep the short one there as it helps to have a minimal example.

[cjerdonek]

Is it possible / would it make sense to also add a test case that corresponds to the example I asked about most recently in the ticket (where the top exception was part of a cycle)?

[cjerdonek]

Does it make sense also to check exc.__context__.__context__ (to check that the cycle was preserved)?

[cjerdonek]

I thought of another case that might be worth testing. What about e.g. A -> B -> C and then raising C while A is active (basically the "Avoid creating new reference cycles" part of your code comment)? I believe that's an example of the kind of case that comment is referencing (or if not, maybe you could add one).

[iritkatriel]

I thought of another case that might be worth testing. What about e.g. A -> B -> C and then raising C while A is active (basically the "Avoid creating new reference cycles" part of your code comment)? I believe that's an example of the kind of case that comment is referencing (or if not, maybe you could add one).

You're right. This test should have been there already because this is old behaviour, but it doesn't seem to be there. I've added it.

[cjerdonek]

Thanks for all your great work on this, @iritkatriel. I approve! 👍

[iritkatriel]

Thank you @cjerdonek.

@pablogsal , @ambv - let's figure out how far back to backport this. According to https://bugs.python.org/issue25782#msg399281 it seems like 3.9?

[miss-islington]

Thanks @iritkatriel for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

[bedevere-bot]

GH-27706 is a backport of this pull request to the 3.10 branch.

[bedevere-bot]

GH-27707 is a backport of this pull request to the 3.9 branch.