Skip to content

gh-133261: Increase C stack margin #133401

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

Closed
wants to merge 1 commit into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented May 4, 2025

Fix a random crash in test_frame.test_repr_deep() on x86-64.

Fix a random crash in test_frame.test_repr_deep() on x86-64.
@vstinner
Copy link
Member Author

vstinner commented May 4, 2025

@markshannon: Does it look reasonable to you?

@markshannon
Copy link
Member

Rather than increasing the multiple of PYOS_STACK_MARGIN_BYTES that we use, I think it would be better to increase PYOS_STACK_MARGIN_BYTES itself.
PYOS_STACK_MARGIN_BYTES should the safe margin between the soft limit and the hard limit and between the hard limit and the real hardware limit.

We already increased it for Windows debug builds, so let's increase it for non-debug and linux builds to match.
Change #define PYOS_LOG2_STACK_MARGIN 11 to #define PYOS_LOG2_STACK_MARGIN 12.

@vstinner
Copy link
Member Author

vstinner commented May 5, 2025

Change #define PYOS_LOG2_STACK_MARGIN 11 to #define PYOS_LOG2_STACK_MARGIN 12.

It doesn't work: test_frame still crash. I increased PYOS_LOG2_STACK_MARGIN up to 14, it does still crash.

You should be able to reproduce to reproduce the crash on Linux using a release build. Just run test_frame a few times and you get a crash.

@vstinner
Copy link
Member Author

vstinner commented May 5, 2025

Without this change, test_frame does crash after a few attempts.

With this change, it does no longer crash:

$ ./python -u -m test -j2 -F test_frame 
...
0:01:00 load avg: 3.31 [ 65] test_frame passed
0:01:00 load avg: 3.31 [ 66] test_frame passed
^C

@vstinner
Copy link
Member Author

vstinner commented May 5, 2025

This change is not needed.

@vstinner vstinner closed this May 5, 2025
@vstinner vstinner deleted the c_stack_soft_limit branch May 5, 2025 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants