Skip to content

ci: fix free-threading issue in test_coroutine #5069

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
merged 1 commit into from
Apr 21, 2025

Conversation

davidhewitt
Copy link
Member

@davidhewitt davidhewitt commented Apr 11, 2025

Three tests in test_coroutine all import the __main__ module and then execute some code which defines a symbol main().

This is problematic on the free-threaded build because the definition of def main() and then the following call to asyncio.run() can race with the other tests overwriting the main symbol with their own implementation of it.

This can lead to the wrong code getting called in the tests & unexpected failures!

cc @ngoldbaum - no issues in CPython or PyO3 to worry about, just a flaky test suite 😂

@davidhewitt davidhewitt requested a review from ngoldbaum April 11, 2025 19:14
@davidhewitt davidhewitt mentioned this pull request Apr 11, 2025
@ngoldbaum
Copy link
Contributor

The fix makes sense to me. I'm pretty sure the codecov failure is spurious.

Teeny style cleanup suggestion - probably for a followup - is to edit all these so they call the Python token py instead of gil - gil is unidiomatic compared with the rest of the codebase.

± rg "with_gil\(\|gil"
src/impl_/coroutine.rs
53:        Python::with_gil(|gil| {
90:        Python::with_gil(|gil| {

src/coroutine/waker.rs
44:        Python::with_gil(|gil| {

guide/src/async-await.md
69:        Python::with_gil(|gil| {

tests/test_coroutine.rs
35:    Python::with_gil(|gil| {
61:    Python::with_gil(|gil| {
96:    Python::with_gil(|gil| {
115:    Python::with_gil(|gil| {
129:    Python::with_gil(|gil| {
138:    Python::with_gil(|gil| {
177:    Python::with_gil(|gil| {
209:    Python::with_gil(|gil| {
237:    Python::with_gil(|gil| {
287:    Python::with_gil(|gil| {
345:    Python::with_gil(|gil| {

@davidhewitt davidhewitt added this pull request to the merge queue Apr 21, 2025
Merged via the queue into PyO3:main with commit b574650 Apr 21, 2025
44 of 45 checks passed
@davidhewitt davidhewitt deleted the async-tests-free-threading branch April 21, 2025 10:02
ngoldbaum pushed a commit to clin1234/pyo3 that referenced this pull request Apr 25, 2025
github-merge-queue bot pushed a commit that referenced this pull request Apr 26, 2025
* Add news item

* Move news file

* Fix version limit check in noxfile.py

* Bump Python version for testing debug builds

* 3.14 is available from GH's setup-python action

* Bump maximum supported CPython version in pyo3-ffi

* Rework PyASCIIObject and PyUnicodeObject to be compatible with 3.14

Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.

* Run `cargo fmt --all`

* Actually add Py_3_14 as a legitimate macro

When `rustc` is invoked, the macro is included with the `--check-cfg`
flag, but not with the `--cfg` flag. This caused errors about duplicate
definitions to spew out when building with stable Rust toolchains.

* Revert "Actually add Py_3_14 as a legitimate macro"

This reverts commit 5da57af.

* Fix version macro placement for 3.14-specific getters and setters

* Import 'c_ushort' only if compiling against CPython 3.14 or later

* Add wrapper functions for the statically_allocated field

* Remove unused libc::c_ushort

* Add (hopefully) final version-specific macros

* Port 3.14-specific 64-bit code of Py_INCREF

* Don't expose PyDictObject.ma_version_tag when building against 3.14 or later

* fix ffi-check on the GIL-enabled ABI

* fix older pythons

* fix ffi-check on older pythons

* WIP: update for 3.14t

* fix ffi-check on the free-threaded build

* fix clippy

* fix clippy on older python versions

* fix cargo check on the MSRV

* fix ffi-check on 3.13t

* fix CI which is using 3.13.1

* fix copy/paste error in noxfile

* update ffi bindings for the latest changes in 3.14

* update layout of refcnt field on gil-enabled build

* delete unused HangThread struct

* fix ffi-check on GIL-enabled build

* Revert "delete unused HangThread struct"

This reverts commit 3dd439d.

* config-out HangThread

* fix 3.13 ffi-check

* fix debug python build error

* fix graalpy build

* Ignore DeprecationWarnings from the  pytest_asyncio module in tests

* Add abi3-py314

* fix free-threading issue in `test_coroutine` (#5069)

* Introspection: add function signatures (#5025)

* Introspection: add function signatures

No annotations or explicit default values yet

Fixes an issue related to object identifiers path

* Better default value

* Refine arguments struct

* Introduce VariableLengthArgument

* Adds pyfunctions tests

* Adds some serialization tests

* respond to david's code review

* add comment and fix test failure

* fix check-feature-powerset

* fix clippy

* fix wasip1 clippy

* fix 32 bit python 3.14 bug

* mark test-py step continue-on-error for dev python builds

* use github issue URL

* run ffi-check before running tests

* fix ffi-check for 3.14.0a7

---------

Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
Co-authored-by: David Hewitt <mail@davidhewitt.dev>
Co-authored-by: Thomas Tanon <thomas.pellissier-tanon@helsing.ai>
newcomertv pushed a commit to newcomertv/pyo3 that referenced this pull request Apr 28, 2025
newcomertv pushed a commit to newcomertv/pyo3 that referenced this pull request Apr 28, 2025
* Add news item

* Move news file

* Fix version limit check in noxfile.py

* Bump Python version for testing debug builds

* 3.14 is available from GH's setup-python action

* Bump maximum supported CPython version in pyo3-ffi

* Rework PyASCIIObject and PyUnicodeObject to be compatible with 3.14

Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.

* Run `cargo fmt --all`

* Actually add Py_3_14 as a legitimate macro

When `rustc` is invoked, the macro is included with the `--check-cfg`
flag, but not with the `--cfg` flag. This caused errors about duplicate
definitions to spew out when building with stable Rust toolchains.

* Revert "Actually add Py_3_14 as a legitimate macro"

This reverts commit 5da57af.

* Fix version macro placement for 3.14-specific getters and setters

* Import 'c_ushort' only if compiling against CPython 3.14 or later

* Add wrapper functions for the statically_allocated field

* Remove unused libc::c_ushort

* Add (hopefully) final version-specific macros

* Port 3.14-specific 64-bit code of Py_INCREF

* Don't expose PyDictObject.ma_version_tag when building against 3.14 or later

* fix ffi-check on the GIL-enabled ABI

* fix older pythons

* fix ffi-check on older pythons

* WIP: update for 3.14t

* fix ffi-check on the free-threaded build

* fix clippy

* fix clippy on older python versions

* fix cargo check on the MSRV

* fix ffi-check on 3.13t

* fix CI which is using 3.13.1

* fix copy/paste error in noxfile

* update ffi bindings for the latest changes in 3.14

* update layout of refcnt field on gil-enabled build

* delete unused HangThread struct

* fix ffi-check on GIL-enabled build

* Revert "delete unused HangThread struct"

This reverts commit 3dd439d.

* config-out HangThread

* fix 3.13 ffi-check

* fix debug python build error

* fix graalpy build

* Ignore DeprecationWarnings from the  pytest_asyncio module in tests

* Add abi3-py314

* fix free-threading issue in `test_coroutine` (PyO3#5069)

* Introspection: add function signatures (PyO3#5025)

* Introspection: add function signatures

No annotations or explicit default values yet

Fixes an issue related to object identifiers path

* Better default value

* Refine arguments struct

* Introduce VariableLengthArgument

* Adds pyfunctions tests

* Adds some serialization tests

* respond to david's code review

* add comment and fix test failure

* fix check-feature-powerset

* fix clippy

* fix wasip1 clippy

* fix 32 bit python 3.14 bug

* mark test-py step continue-on-error for dev python builds

* use github issue URL

* run ffi-check before running tests

* fix ffi-check for 3.14.0a7

---------

Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
Co-authored-by: David Hewitt <mail@davidhewitt.dev>
Co-authored-by: Thomas Tanon <thomas.pellissier-tanon@helsing.ai>
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