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

Incorrect #[cfg] before PyEval_InitThreads breaks Python::acquire_gil() in standalone scripts on <= 3.6 #219

Closed
joar opened this issue Sep 11, 2018 · 2 comments

Comments

@joar
Copy link
Contributor

joar commented Sep 11, 2018

🐛 Bug Reports

When reporting a bug, please provide the following information. If this is not a bug report you can just discard this template.

🌍 Environment

  • Your operating system and version: Ubuntu 18.04
  • Your python version: Python 3.6 in this case
  • How did you install python (e.g. apt or pyenv)? Did you use a virtualenv?:
    • Python is installed via apt.
    • I use python's venv.
  • Your rust version (rustc --version): rustc 1.30.0-nightly (2d4e34ca8 2018-09-09)

💥 Reproducing

Check out the code in the fork at https://github.com/joar/pyo3/tree/bug/pyeval-threadsnotinitialized/examples/verify_gil.

cd examples/verify_gil

if you haven't already

$ python -VV
Python 3.6.5 (default, Apr  1 2018, 05:46:30) 
[GCC 7.3.0]

$ python setup.py develop
[...]

$ cat ../../target/debug/build/pyo3-4833970e0e0d17d0/output
cargo:rustc-cfg=Py_3_5
cargo:rustc-cfg=Py_3_6
cargo:rustc-cfg=Py_3
cargo:rustc-cfg=py_sys_config="WITH_THREAD"
cargo:python_flags=FLAG_WITH_THREAD=1,CFG_Py_3_5,CFG_Py_3_6
cargo:rerun-if-env-changed=LD_LIBRARY_PATH
cargo:rerun-if-env-changed=PATH
cargo:rerun-if-env-changed=PYTHON_SYS_EXECUTABLE
cargo:rerun-if-env-changed=LIB

$ python naked-script.py
thread '<unnamed>' panicked at 'assertion failed: `(left != right)`
  left: `0`,
 right: `0`', src/pythonrun.rs:42:13
note: Run with `RUST_BACKTRACE=1` for a backtrace.
fatal runtime error: failed to initiate panic, error 5
fish: “python naked-script.py” terminated by signal SIGABRT (Abort)

Note: Importing the script via ipython still works. I supect that is because some other extension that ipython uses calls PyEval_InitThreads.

🎉 Solution?

It think that #[cfg(py_sys_config = "WITH_THREAD")] always is false even if build.rs sets cargo:rustc-cfg=py_sys_config="WITH_THREAD". I think the solution might be to just change to #[cfg(not(Py_3_7))].

@joar
Copy link
Contributor Author

joar commented Sep 11, 2018

Actually, the issue is probably that the --cfg from pyo3 aren't set for the pyo3-derive-backend crate.

Although it seems like Python 3.7 survives a call to PyEval_InitThreads in my case.

@konstin
Copy link
Member

konstin commented Sep 11, 2018

Thank you for this great bug report! The problem is indeed that the cfg isn't set so doesn't get called PyEval_InitThreads. (The code that is called is part of the generated code for the #[pymodule], so it's in the verify-gil crate rather than in the backend, but the point stands either way).

Why Python 3.7 works is explained in the c-api docs:

This is a no-op when called for a second time.

Changed in version 3.7: This function is now called by Py_Initialize(), so you don’t have to call it yourself anymore.

This means you can still call PyEval_InitThreads in 3.7, it's just not required anymore.

I've fixed this 10ef6cd by moving the call into pyo3. Just adding #[cfg(not(Py_3_7))] (or no cfg guard at all) would work for the most users, but before 3.7 it's supported to compile python without threading and we want to support those versions as well.

joar added a commit to joar/rust-csv-py that referenced this issue Sep 12, 2018
Which contains the fix for PyO3/pyo3#219

Add and test example scripts
konstin added a commit that referenced this issue Sep 17, 2018
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

No branches or pull requests

2 participants