Skip to content

gh-126366: Fix crash if __iter__ raises an exception during yield from #126369

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

Merged
merged 5 commits into from
Nov 5, 2024

Conversation

ZeroIntensity
Copy link
Member

@ZeroIntensity ZeroIntensity commented Nov 3, 2024

Apparently, gh-126366 has two issues: one is a thread safety problem inside list.__init__, and then the other is that the GET_YIELD_FROM_ITER instruction crashes if a non-native-generator object raises an exception in its __iter__. Since the latter only applies to 3.14, I'm splitting the fix into two PRs.

@ZeroIntensity
Copy link
Member Author

I don't understand how the JIT failures could be related.

@ZeroIntensity
Copy link
Member Author

@savannahostrowski @diegorusso My JIT experts, are these known failures, or is there something wrong with my fix?

@savannahostrowski
Copy link
Member

savannahostrowski commented Nov 3, 2024

@ZeroIntensity The aarch64 failures are known/expected. This is what we were discussing over Discord a couple of days back. Right now, we are waiting for this to be fixed in the aarch64 macos runners.

As for the emulated Linux failures, sometimes we do see failures here, and that's why we maintain this list of tests to skip. I will note that this test was seen recently passing in https://github.com/python/cpython/actions/runs/11647994504/job/32434030552?pr=126339).

If this PR has nothing to do with test.test_subprocess.POSIXProcessTestCase.test_vfork_used_when_expected (and I suspect it doesn't...), it may need to be added to the skip list.

@ZeroIntensity
Copy link
Member Author

Thank you, Savannah! I'll update the skip list and see if that fixes it.

Copy link
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

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

LGTM

@kumaraditya303 kumaraditya303 merged commit 1371295 into python:main Nov 5, 2024
65 of 67 checks passed
@vstinner
Copy link
Member

vstinner commented Nov 6, 2024

Would it be possible to backport the test to 3.12 and 3.13?

@ZeroIntensity
Copy link
Member Author

Maybe, but would it be all that useful? Anything that gets backported to those versions will (probably) have to go through the test on main anyway.

@vstinner
Copy link
Member

vstinner commented Nov 6, 2024

It's useful to check for non-regression: make sure that we don't break this code path in the future.

picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
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.

6 participants