Skip to content

gh-136394: Fix race condition in test_zstd #136432

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

Conversation

Rogdham
Copy link
Contributor

@Rogdham Rogdham commented Jul 8, 2025

Apply a suggestion of @emmatyping to solve the race condition identified in gh-136394:

one solution would be to call .compress in the main thread before other threads run so that the other threads should generate identical content and thread ordering shouldn't matter.

Needs backport to 3.14 only.

@ZeroIntensity
Copy link
Member

!buildbot nogil

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @ZeroIntensity for commit 81a1bd7 🤖

Results will be shown at:

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

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

The builders matched are:

  • AMD64 Windows PGO NoGIL PR
  • PPC64LE Fedora Rawhide NoGIL refleaks PR
  • AMD64 Windows Server 2022 NoGIL PR
  • AMD64 Fedora Rawhide NoGIL refleaks PR
  • AMD64 CentOS9 NoGIL PR
  • s390x Fedora Rawhide NoGIL PR
  • s390x Fedora Rawhide NoGIL refleaks PR
  • x86-64 MacOS Intel NoGIL PR
  • x86-64 MacOS Intel ASAN NoGIL PR
  • AMD64 CentOS9 NoGIL Refleaks PR
  • aarch64 Fedora Rawhide NoGIL PR
  • PPC64LE Fedora Rawhide NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • AMD64 Fedora Rawhide NoGIL PR
  • ARM64 MacOS M1 NoGIL PR
  • aarch64 Fedora Rawhide NoGIL refleaks PR

Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

Buildbots are happy!

@emmatyping
Copy link
Member

I also ran the tests 5000 times with this change and found no failures

@Rogdham
Copy link
Contributor Author

Rogdham commented Jul 10, 2025

I also checked in the conditions in which I was able to trigger the issue within a few dozen tries on my machine before the PR, and the issue did not show within 5000 tries with the code from this PR.

So I would say that this fix works as indented, your idea @emmatyping was a good call!

@ZeroIntensity ZeroIntensity merged commit f519918 into python:main Jul 10, 2025
43 checks passed
@miss-islington-app
Copy link

Thanks @Rogdham for the PR, and @ZeroIntensity for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 10, 2025
(cherry picked from commit f519918)

Co-authored-by: Rogdham <3994389+Rogdham@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Jul 10, 2025

GH-136506 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jul 10, 2025
ZeroIntensity pushed a commit that referenced this pull request Jul 10, 2025
)

gh-136394: Fix race condition in test_zstd (GH-136432)
(cherry picked from commit f519918)

Co-authored-by: Rogdham <3994389+Rogdham@users.noreply.github.com>
@Rogdham Rogdham deleted the fix-test_compress_locking-race-condition branch July 10, 2025 14:06
AndPuQing pushed a commit to AndPuQing/cpython that referenced this pull request Jul 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants