mgorny
commented
3 months ago CC @encukou, @ericsnowcurrently in case they have any insight what's wrong.
Closed mgorny closed 3 months ago
mgorny
commented
3 months ago CC @encukou, @ericsnowcurrently in case they have any insight what's wrong.
encukou
commented
3 months ago Thank you for testing with assertions!
It looks like Rust code is increasing the refcount of a static immortal string object (the Python string "__all__").
Not being familiar with Rust I haven't found the source; in GDB you can find it here:
(gdb) watch ((PyObject*)(&_PyRuntime.static_objects.singletons.strings.identifiers._py___all__)).ob_refcnt
Hardware watchpoint 1: [...]
(gdb) run
Starting program: /venv/bin/python -c import\ pyproject_fmt_rust
[...]
Hardware watchpoint 1: ((PyObject*)(&_PyRuntime.static_objects.singletons.strings.identifiers._py___all__)).ob_refcnt
Old value = 4294967295
New value = 4294967296
0x00007f9072ab1cc5 in <pyo3::instance::Bound<pyo3::types::module::PyModule> as pyo3::types::module::PyModuleMethods>::index ()
from /pyproject-fmt-rust/src/pyproject_fmt_rust/_lib.abi3.soA stable ABI extension is allowed to do that (before 3.12 stable ABI; this uses 3.8 AFAIK). The assertion in CPython is wrong. I'll send a CPython PR soon.
davidhewitt
commented
3 months ago Thanks for investigating @encukou!
I think it's correct in your analysis that we are calling Py_INCREF on the __all__ singleton (via our generic code for __getattr__, it looks like). That we do that at all is a performance optimization yet to be resolved.
PyO3 0.21.2 doesn't support the version-specific Python 3.13 API, so it should be the case that this has been built using ABI3 "forward compatibility".
encukou
commented
3 months ago Immortal objects are an implementation detail, so ideally you should call Py_IncRef on "__all__". But you should call it, as a library function, rather than increase ob_refcount directly.
Except if you're using stable ABI. Then you can adjust ob_refcount directly, for backwards compatibility reasons. (Don't do it unless you have to, though...)
(Outside the stable ABI, you could instead reimplement the Py_INCREF macro in Rust, and call that. But in that case include the immortality check, and note that it might change in the next feature release.)
mgorny
commented
3 months ago Thank you for investigating, and for the pull request. I'm going to test it today.
encukou
commented
3 months ago The CPython PR is up at https://github.com/python/cpython/pull/121358. It fixes the reproducer above, at least on my machine :)
davidhewitt
commented
3 months ago Immortal objects are an implementation detail, so ideally you should call
Py_IncRefon"__all__". But you should call it, as a library function, rather than increaseob_refcountdirectly.Except if you're using stable ABI. Then you can adjust
ob_refcountdirectly, for backwards compatibility reasons. (Don't do it unless you have to, though...)(Outside the stable ABI, you could instead reimplement the
Py_INCREFmacro in Rust, and call that. But in that case include the immortality check, and note that it might change in the next feature release.)
Yep that's pretty much exactly the situation. By the optimization I meant in this case I think PyO3 shouldn't need to call Py_INCREF on the attribute name to be passed to PyObject_GetAttr (as that API doesn't steal a reference). But unrelated to the main point here wrt the assertion which PyO3 will still hit in the general case.
davidhewitt
commented
3 months ago I think I will close this one as we understand the fix to be coming in 3.13b4. Thanks @mgorny @encukou!
Bug Description
After upgrading to Python 3.13.0b3, various projects that previously worked with Python 3.13.0b2 suddenly started crashing with assertion error. For example,
pyproject-fmt-rust:I've been able to bisect it to python/cpython@9769b7ae064a0546a98cbcbec2561dbaba20cd23. Unfortunately, I've been only able to test it against pyo3-0.21.2, as porting that package to 0.22.0 is above my current skills. However, a quick search through the repository doesn't reveal anything obviously relevant. If I missed something, I'm really sorry about that.
Steps to Reproduce
--with-assertions.git clone https://github.com/tox-dev/pyproject-fmt-rust/cd pyproject-fmt-rust~/git/cpython/python -m venv .venv,. .venv/bin/activate).pip install -e .python -c "import pyproject_fmt_rust"Backtrace
Your operating system and version
Gentoo Linux amd64
Your Python version (
python --version)Python 3.13.0b3
Your Rust version (
rustc --version)rustc 1.79.0 (129f3b996 2024-06-10) (gentoo)
Your PyO3 version
0.21.2
How did you install python? Did you use a virtualenv?
Reproduced both with Python build from source with
--with-assertions(as part ofgit bisect) and 3.13.0b3 from Gentoo ebuild. Virtualenv for testing as noted above.Additional Info
No response