Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset exception in WithCleanupFinish #5203

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

dchiquito
Copy link
Contributor

Context managers have an __exit__ function that returns a boolean-like object. If the object is truthy, then exceptions are suppressed.

If an exception was thrown while resolving that boolean, it would leak and live on in the error stack, getting tacked on to all future exceptions. This caused several mysterious test failures which would only trigger after this very specific event was tested in test_with.

The solution is to move a call to vm.set_exception() before attempting the try_to_bool() which threw the error.

Minimal example to reproduce the bug:

import sys
import traceback

class cm(object):
    def __init__(self):
        pass

    def __enter__(self):
        return 3

    def __exit__(self, a, b, c):
        class Bool:
            def __bool__(self):
                1 // 0
        return Bool()

try:
    with cm():
        raise Exception("Should NOT see this")
except ZeroDivisionError:
    print("exception caught, as expected")

print("There should now be no exception")
traceback.print_exc()
print(sys.exc_info())

Context managers have an `__exit__` function that returns a boolean-like
object. If the object is truthy, then exceptions are suppressed.

If an exception was thrown while resolving that boolean, it would leak
and live on in the error stack, getting tacked on to all future
exceptions. This caused several mysterious test failures which would
only trigger after this very specific event was tested in `test_with`.

The solution is to move a call to `vm.set_exception()` before
attempting the `try_to_bool()` which threw the error.

Minimal example to reproduce the bug:
```py
import sys
import traceback

class cm(object):
    def __init__(self):
        pass

    def __enter__(self):
        return 3

    def __exit__(self, a, b, c):
        class Bool:
            def __bool__(self):
                1 // 0
        return Bool()

try:
    with cm():
        raise Exception("Should NOT see this")
except ZeroDivisionError:
    print("exception caught, as expected")

print("There should now be no exception")
traceback.print_exc()
print(sys.exc_info())
```
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@youknowone youknowone merged commit 85c427b into RustPython:main Mar 21, 2024
11 checks passed
@youknowone
Copy link
Member

Thank you so much! This fix seems hard to find the place to fix but related to many with bugs!

@dchiquito dchiquito deleted the fix-cm-cleanup branch March 21, 2024 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants