gh-116098: Clean up frame allocation code and remove the invalid sneaky frame test by gaogaotiantian · Pull Request #116687 · python/cpython · GitHub
Skip to content

gh-116098: Clean up frame allocation code and remove the invalid sneaky frame test - #116687

Merged
brandtbucher merged 2 commits into
python:mainfrom
gaogaotiantian:remove-sneaky-frame-test
Mar 12, 2024
Merged

brandtbucher merged 2 commits into
python:mainfrom
gaogaotiantian:remove-sneaky-frame-test

Conversation

@gaogaotiantian

@gaogaotiantian gaogaotiantian commented Mar 12, 2024

Copy link
Copy Markdown
Member

After a discussion with @brandtbucher , we believe the test is not testing what it's supposed to anymore. No new frame is created during the frame allocation and the test lost its meaning. Under no circumstance this test is valid or checking something reasonable. So we should just remove this test.

The fundamental reason the test is invalid is that the current frame allocation code is immune from being interrupted by Python code that can somehow create the same frame object. So we also remove the dead code in frame allocation, and replace it with an assert and some comments.

There's no user observable behavior changes so I'll skip the news, unless someone disagrees.

ghost left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

Comment thread Python/frame.c Outdated
@brandtbucher
brandtbucher enabled auto-merge (squash) March 12, 2024 22:50
@brandtbucher brandtbucher added tests Tests in the Lib/test dir interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Mar 12, 2024
@brandtbucher brandtbucher self-assigned this Mar 12, 2024
@brandtbucher

ghost commented Mar 12, 2024

Copy link
Copy Markdown
Member

Comment thread Python/frame.c

ghost Sep 18, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should also remove the corresponding assert(frame->owner != FRAME_CLEARED) statements, especially since FRAME_CLEARED was never a valid member of enum _frameowner (it’s from enum _framestate).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

interpreter-core (Objects, Python, Grammar, and Parser dirs) skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants