GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [3.11] gh-106883: Make test_current_frames_exceptions_deadlock faster by diegorusso · Pull Request #131182 · python/cpython

Conversation

@diegorusso

@diegorusso diegorusso commented Mar 13, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

On some platforms the test test_current_frames_exceptions_deadlock goes in timeout when CPython is built with debug.
The execution time of the test is about 4x and the current timeout is not enough to complete the test.

Make the test test_current_frames_exceptions_deadlock faster by decrementing the number of threads to the bare minimum and the number of objects by one order of magnitude.
These conditions are enough for a deadlock to happen.
Make use of support.SHORT_TIMEOUT which is 30 seconds by default.

  • Issue: Deadlock in threaded application when using sys._current_frames #106883

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@diegorusso
[3.11] pythongh-106883: Increase timeout for a test_sys test
2bd561d 
On some platforms the test test_current_frames_exceptions_deadlock
goes in timeout when CPython is built with debug.
The execution time of the test is about 4x and the current timeout is
not enough to complete the test.
@diegorusso
diegorusso requested a review from pablogsal March 13, 2025 11:29
@bedevere-app bedevere-app Bot added tests Tests in the Lib/test dir awaiting review labels Mar 13, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Mar 13, 2025
Deadlock in threaded application when using sys._current_frames #106883
Closed
@diegorusso diegorusso mentioned this pull request Mar 13, 2025
[3.11] gh-106883 Fix deadlock in threaded application #117332
Merged
@diegorusso diegorusso added skip news 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels Mar 13, 2025
@bedevere-bot

bedevere-bot commented Mar 13, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @diegorusso for commit 2bd561d 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131182%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 13, 2025
@python python deleted a comment from bedevere-bot Mar 13, 2025
@diegorusso

diegorusso commented Mar 13, 2025

Copy link
Copy Markdown
Contributor Author

!buildbot ppc64le
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Mar 13, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @diegorusso for commit 2bd561d 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131182%2Fmerge

The command will test the builders whose names match following regular expression: ppc64le

The builders matched are:

  • PPC64LE RHEL8 LTO + PGO PR
  • PPC64LE Fedora Rawhide Clang PR
  • PPC64LE Fedora Rawhide NoGIL PR
  • PPC64LE Fedora Rawhide PR
  • PPC64LE CentOS9 PR
  • PPC64LE Fedora Rawhide Refleaks PR
  • PPC64LE Fedora Stable LTO + PGO PR
  • PPC64LE Fedora Rawhide Clang Installed PR
  • PPC64LE Fedora Stable PR
  • PPC64LE Fedora Rawhide NoGIL refleaks PR
  • PPC64LE CentOS9 LTO + PGO PR
  • PPC64LE CentOS9 LTO PR
  • PPC64LE Fedora Rawhide LTO PR
  • PPC64LE Fedora Stable Refleaks PR
  • PPC64LE Fedora Stable Clang PR
  • PPC64LE RHEL8 LTO PR
  • PPC64LE Fedora Stable LTO PR
  • PPC64LE CentOS9 Refleaks PR
  • PPC64LE RHEL8 Refleaks PR
  • PPC64LE RHEL8 PR
  • PPC64LE Fedora Stable Clang Installed PR
  • PPC64LE Fedora Rawhide LTO + PGO PR
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
colesbury
colesbury reviewed Mar 13, 2025
View reviewed changes
Comment thread Lib/test/test_sys.py Outdated
# hence the test fails.
# The timeout is high because on older platforms it takes some time
# for the test to be executed when CPython is built with debug flags.
TIMEOUT = 160

@colesbury colesbury Mar 13, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

  1. Consider adding @support.requires_resource("cpu") if this test can take a long time to run.
  2. You should use support.SHORT_TIMEOUT or support.LONG_TIMEOUT, which scales with --timeout, so it's longer on slow buildbots.

SHORT_TIMEOUT: It's 30 seconds for make test and anywhere from 60 seconds - 135 seconds on the three failing buildbots.

LONG_TIMEOUT: 10x SHORT_TIMEOUT.

If SHORT_TIMEOUT isn't enough, you should really consider making the test faster!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
diegorusso reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@diegorusso diegorusso Mar 13, 2025

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks for the suggestions. I've actually found a way to make it faster :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@diegorusso diegorusso Mar 13, 2025

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Done, let's see how it goes.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@diegorusso
Address feedback
abf4144
@diegorusso

diegorusso commented Mar 13, 2025

Copy link
Copy Markdown
Contributor Author

!buildbot ppc64le
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Mar 13, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @diegorusso for commit abf4144 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131182%2Fmerge

The command will test the builders whose names match following regular expression: ppc64le

The builders matched are:

  • PPC64LE RHEL8 LTO + PGO PR
  • PPC64LE Fedora Rawhide Clang PR
  • PPC64LE Fedora Rawhide NoGIL PR
  • PPC64LE Fedora Rawhide PR
  • PPC64LE CentOS9 PR
  • PPC64LE Fedora Rawhide Refleaks PR
  • PPC64LE Fedora Stable LTO + PGO PR
  • PPC64LE Fedora Rawhide Clang Installed PR
  • PPC64LE Fedora Stable PR
  • PPC64LE Fedora Rawhide NoGIL refleaks PR
  • PPC64LE CentOS9 LTO + PGO PR
  • PPC64LE CentOS9 LTO PR
  • PPC64LE Fedora Rawhide LTO PR
  • PPC64LE Fedora Stable Refleaks PR
  • PPC64LE Fedora Stable Clang PR
  • PPC64LE RHEL8 LTO PR
  • PPC64LE Fedora Stable LTO PR
  • PPC64LE CentOS9 Refleaks PR
  • PPC64LE RHEL8 Refleaks PR
  • PPC64LE RHEL8 PR
  • PPC64LE Fedora Stable Clang Installed PR
  • PPC64LE Fedora Rawhide LTO + PGO PR
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@diegorusso diegorusso changed the title [3.11] gh-106883: Increase timeout for a test_sys test [3.11] gh-106883: Make test_current_frames_exceptions_deadlock faster Mar 13, 2025
@diegorusso
diegorusso requested a review from colesbury March 13, 2025 15:13
colesbury
colesbury approved these changes Mar 13, 2025
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
diegorusso reacted with hooray emoji
All reactions
  • 🎉 1 reaction
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Mar 13, 2025
@diegorusso

diegorusso commented Mar 13, 2025

Copy link
Copy Markdown
Contributor Author

@pablogsal have a look. All the buildbot failures are not related to this PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal approved these changes Mar 14, 2025
View reviewed changes
@pablogsal

pablogsal commented Mar 14, 2025

Copy link
Copy Markdown
Member

LGTM

Thanks a lot @diegorusso for the investigation and @colesbury for the review!
diegorusso reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal
pablogsal merged commit f576d31 into python:3.11 Mar 14, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Mar 14, 2025
@diegorusso
diegorusso deleted the gh-106883-timeout branch April 4, 2025 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colesbury colesbury colesbury approved these changes
@pablogsal pablogsal pablogsal approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@diegorusso @bedevere-bot @pablogsal @colesbury
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.