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

Fix segmentation fault and FrameLocalsProxy check #328

Merged
merged 2 commits into from
Oct 4, 2024

Conversation

dataflake
Copy link
Member

@dataflake dataflake commented Oct 3, 2024

Related to #323

The original issue, a segmentation fault, was fixed between Python 3.13rc2 and rc3. This PR applies a proposed patch by @vstinner in #323 (comment) to fix a followup issue.

@vstinner
Copy link

vstinner commented Oct 3, 2024

The original issue, a segmentation fault, was fixed between Python 3.13rc2 and rc3. This PR applies a proposed patch by @vstinner in #323 (comment) to fix a followup issue.

Right, even if the segfault was fixed, the test still fails because the constructor expects an argument (a frame).

Copy link
Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

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

It took me a minute to study how these tests work and why this is needed. They automatically verify that subclasses of the abstract base classes in collections.abc conform to the corresponding interfaces. And FrameLocalsProxy is a subclass of collections.abc.Mapping.

@davisagli
Copy link
Member

davisagli commented Oct 3, 2024

@dataflake The issue fixed here is unrelated to the original issue that was reported in #323 (a different segmentation fault that still has a fix pending in #324). So I will edit the PR description to make sure that merging this does not close that issue.

@dataflake dataflake merged commit d16f663 into master Oct 4, 2024
54 checks passed
@dataflake dataflake deleted the dataflake/py_313_crashes branch October 4, 2024 08:43
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.

4 participants