Skip to content
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

Try fix CI #2314

Merged
merged 1 commit into from
Apr 19, 2022
Merged

Try fix CI #2314

merged 1 commit into from
Apr 19, 2022

Conversation

mejrs
Copy link
Member

@mejrs mejrs commented Apr 18, 2022

The Python 3.11 runner keeps failing:

failures:

---- test_anext_aiter stdout ----
thread 'test_anext_aiter' panicked at 'called `Result::unwrap()` on an `Err` value: ()', tests\test_proto_methods.rs:779:10
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\/library\std\src\panicking.rs:584
   1: core::panicking::panic_fmt
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\/library\core\src\panicking.rs:143
   2: core::result::unwrap_failed
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\/library\core\src\result.rs:1749
   3: core::result::Result::unwrap<tuple$<>,tuple$<> >
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\library\core\src\result.rs:1065
   4: test_proto_methods::test_anext_aiter
             at .\tests\test_proto_methods.rs:777
   5: test_proto_methods::test_anext_aiter::closure$0
             at .\tests\test_proto_methods.rs:750
   6: core::ops::function::FnOnce::call_once<test_proto_methods::test_anext_aiter::closure_env$0,tuple$<> >
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\library\core\src\ops\function.rs:227
   7: core::ops::function::FnOnce::call_once
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\library\core\src\ops\function.rs:227
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    test_anext_aiter

test result: FAILED. 25 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.21s

error: test failed, to rerun pass '--test test_proto_methods'

I have reproduced this locally, using the 3.11.0-alpha.7 interpreter.

It does not happen if either:

  • the previous version, 3.11.0-alpha.6 is used.
  • tests are run single threaded, with e.g. cargo test --test test_proto_methods --features=full -- --test-threads 1

@mejrs mejrs added the CI-no-fail-fast If one job fails, allow the rest to keep testing label Apr 18, 2022
@mejrs
Copy link
Member Author

mejrs commented Apr 18, 2022

It also prints Could not find platform dependent libraries <exec_prefix>, even if it doesn't actually crash.

Edit: Seems like it has been doing this since 3.11 was added to the workflow.

@mejrs
Copy link
Member Author

mejrs commented Apr 18, 2022

This reproduces the error:

use pyo3::prelude::*;

const SOURCE: &str =  r#"
import asyncio
import sys

async def main():
    pass

# For an odd error similar to https://bugs.python.org/issue38563
if sys.platform == "win32" and sys.version_info >= (3, 8, 0):
    asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())

asyncio.run(main())
"#;

fn main() {
    Python::with_gil(|_| {});

    let handle = std::thread::spawn(|| {
        Python::with_gil(|py| {
            py.run(SOURCE, None, None).map_err(|e| e.print(py)).unwrap();
        });
    });

    handle.join().unwrap();
}
Traceback (most recent call last):
  File "<string>", line 12, in <module>
  File "C:\Users\bruno\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 175, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "C:\Users\bruno\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 103, in run
    signal.signal(signal.SIGINT, sigint_handler)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\bruno\AppData\Local\Programs\Python\Python311\Lib\signal.py", line 56, in signal
    handler = _signal.signal(_enum_to_int(signalnum), _enum_to_int(handler))
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: signal only works in main thread of the main interpreter

@mejrs
Copy link
Member Author

mejrs commented Apr 18, 2022

python/cpython#32105

Why is this merged? No core dev reviewed this. Please revert.

😬

I guess we should pin to alpha-6 for now?

@messense
Copy link
Member

I guess we should pin to alpha-6 for now?

To unblock other PRs, I think it makes sense to do so.

@davidhewitt
Copy link
Member

Yep agreed. Thanks for investigating!

@davidhewitt davidhewitt merged commit e88655d into PyO3:main Apr 19, 2022
@gvanrossum
Copy link

I'm curious -- the PR (python/cpython#32105) has an explicit check whether it's running in the main thread:

        if (threading.current_thread() is threading.main_thread()
            and signal.getsignal(signal.SIGINT) is signal.default_int_handler
        ):
            sigint_handler = functools.partial(self._on_sigint, main_task=task)
            signal.signal(signal.SIGINT, sigint_handler)

Is there something unusual in your code that makes the first line of that check pass? Are you using subinterpreters?

@gvanrossum
Copy link

@davidhewitt answered in some detail on the Python issue thread, let's continue the discussion there.

@davidhewitt
Copy link
Member

For completeness, I think the threading check passes because threading does not have a good answer for what the main thread is in this case. python/cpython#32105 (comment)

See also #2319 (comment) where I comment a little on how we call import threading in some situations where we have a good answer what the main thread is.

Aside, it would be cool if we were using sub-interpreters, however that's still miles off because it's a huge research question how we need to change the PyO3 public API to allow us to implement PEP 489 compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-no-fail-fast If one job fails, allow the rest to keep testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants