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

PySet::pop sets the global exception state #688

Closed
nagisa opened this issue Dec 9, 2019 · 0 comments · Fixed by #719
Closed

PySet::pop sets the global exception state #688

nagisa opened this issue Dec 9, 2019 · 0 comments · Fixed by #719
Labels

Comments

@nagisa
Copy link
Contributor

nagisa commented Dec 9, 2019

Discovered by running the test suite locally.

The PySet_Pop that’s run as part of the test_set_pop will return NULL and set the global exception state (raise an exception) when the set is empty. We handle the NULL case and return an Option<T>, but the exception state remains set.

Given that the convention in PyO3 is to convert exceptions to return values, this can be very misleading. In particular, this causes the test suite to fail further along when one of the tests invokes py.eval and then converts this exception to a Result.

To reproduce try: cargo test --lib -- --test-threads=1.

@kngwyu kngwyu added the bug label Dec 10, 2019
Alexander-N added a commit to Alexander-N/pyo3 that referenced this issue Jan 10, 2020
Leaving Python's global exception state is misleading, e.g. subsequent
calls of `py.eval` will fail.

Closes PyO3#688
Alexander-N added a commit to Alexander-N/pyo3 that referenced this issue Jan 10, 2020
Leaving Python's global exception state is misleading, e.g. subsequent
calls of `py.eval` will fail.

Closes PyO3#688
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants