Skip to content

Conversation

davidhewitt
Copy link
Member

This PR avoids races in tests which use warnings.capture_warnings.

While we had issues with Py_GIL_DISABLED with these tests previously, warnings.capture_warnings, not being thread-safe of course means that no two rust tests doing such work can be run concurrently, because even GIL switches can lead to tests interleaving and therefore getting bad results.

While we'd not observed these races on the GIL-enabled builds before, I think #5341 has actually gotten stuck due to this behavior because disallowing races to build types has introduced a new synchronization point between test threads.

I fix that problem in this PR by introducing a mutex into the CatchWarnings test helper; as long as all tests go through that helper they will be forced to run sequentially for the sensitive block. I had to rearrange the "test utils" file a bit so that it was available at crate::test_utils in all places it's used, so that the macros inside it work properly.

As a result, I think I can enable all these tests on the free-threaded build too :)

cc @ngoldbaum

@davidhewitt
Copy link
Member Author

CI seems fully green, I'm going to merge this as a tests-only change. Please forgive 🙏

@davidhewitt davidhewitt added this pull request to the merge queue Aug 23, 2025
Merged via the queue into PyO3:main with commit 2fa818b Aug 23, 2025
44 checks passed
@davidhewitt davidhewitt deleted the test-warnings-races branch August 23, 2025 06:25
@ngoldbaum
Copy link
Contributor

Cool, I'm glad you were able to re-enable a bunch of tests!

@ngoldbaum
Copy link
Contributor

By the way, Python 3.14 and newer has thread-safe warnings. It's turned on by default on the free-threaded build but you need to enable it with a new -X CLI option on the GIL-enabled build. See https://docs.python.org/3.14/whatsnew/3.14.html#concurrent-safe-warnings-control.

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.

2 participants