-
-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
Stable ABI: Objects/bytesobject.c:122: PyBytes_FromStringAndSize: Assertion '_Py_IsImmortal(op)' failed.
#123091
Comments
I think this should get investigated by PyO3 first, but at a very quick glance, this looks like it could be a problem with the latin 1-byte strings string interning. Possibly related to #122303. |
Actually, scratch that: I think it's caused by #120520, which allows interned strings to be mortal, so this might just be a precondition that wasn't updated. Do any other bugs occur if you remove that assertion? |
If you mean that specific test, it passes then. However, the test suite in general triggers another failed assertion:
But I suppose that's a separate problem to report, and ideally I'd try to pinpoint it better first. |
I'm guessing something changed recently with immortality, but this could be a PyO3 issue depending on what APIs they're using. Has this been reported to them yet? |
We've reported similar issues (i.e. regarding immortality changes in 3.13) to them before, and the conclusion was that since they're using the stable ABI here, CPython needs to preserve compatibility for the sake of extensions built against earlier versions. PyO3 already have done a major fix that causes these issues to disappear, but the existing extensions will continue crashing until new wheels are built against newer PyO3 versions (and things move slowly). I've CC-ed David Hewitt here for completeness, but it feels weird to report issues if I know that they are fixed on PyO3's end — so I'm filing them on this end for completeness. |
Is PyO3 itself using the stable ABI, or just |
Objects/bytesobject.c:122: PyBytes_FromStringAndSize: Assertion '_Py_IsImmortal(op)' failed.
Objects/bytesobject.c:122: PyBytes_FromStringAndSize: Assertion '_Py_IsImmortal(op)' failed.
@picnixz this is |
I assume this is indeed one of the cases already "fixed" on PyO3's side by moving to use FFI calls for all refcounting with the stable ABI - as (We released the fix in 0.22.2; I think based on PyO3/pyo3#4428 the qiskit team is working on updating to PyO3 0.22) The main point I see here is that assertions of immortality are broken when compiling against stable ABI versions which used inline reference counting i.e. before immortality was added and the stable API switched to use function calls. |
We are planning to upgrade to pyo3 0.22.x asap, but we're blocked on the next rust-numpy release before we can really start the process as we rely fairly heavily on numpy. |
The string interning changes involve Yes, this looks like another issue like #118997 and #121528. Turns out I guess next week I'll audit all the uses of |
I see at least 2 dependencies installed from stable ABI binaries built with Python 3.8: rustworkx and qiskit. As @encukou wrote, it reminds me issues #118997 and #121528. Maybe it's another place where _Py_IsImmortalLoose() is needed. |
Sure, will get to it right away. |
Well, that PR fixes makes the test pass, but pytest crashes on exit:
|
Thanks for testing! Well, I tried to write a minimum patch. I saw this assertion in typeobject.c but I skipped it. I updated my PR to also patch this assertion and 2 others in typeobject.c. Would you mind to test again with the updated PR? |
One down, more to go:
|
I'm not sure if my "whack-a-mole" approach makes sense :-( Well, I updated my PR to change also ceval.c. Remaining unpatched code:
|
Yeah, makes you wonder if we'd not eventually end up replacing all of them. However, at least this won't happen with qiskit — the newest version of the PR seems to resolve all the crashes (I had to redo the last commit for 3.13). |
Use _Py_IsImmortalLoose() in bytesobject.c, typeobject.c and ceval.c.
Use _Py_IsImmortalLoose() in bytesobject.c, typeobject.c and ceval.c. (cherry picked from commit f1a0d96)
Good. I merged my PR. I also created a backport to Python 3.13: PR gh-123600. |
Thanks a lot! |
I sent PR #123602 for the remaining ones. |
Switch more _Py_IsImmortal(...) assertions to _Py_IsImmortalLoose(...) The remaining calls to _Py_IsImmortal are in free-threaded-only code, initialization of core objects, tests, and guards that fall back to code that works with mortal objects.
Switch more _Py_IsImmortal(...) assertions to _Py_IsImmortalLoose(...) The remaining calls to _Py_IsImmortal are in free-threaded-only code, initialization of core objects, tests, and guards that fall back to code that works with mortal objects. (cherry picked from commit 57c471a)
Switch more _Py_IsImmortal(...) assertions to _Py_IsImmortalLoose(...) The remaining calls to _Py_IsImmortal are in free-threaded-only code, initialization of core objects, tests, and guards that fall back to code that works with mortal objects. (cherry picked from commit 57c471a)
Thanks to everyone involved! |
Use _Py_IsImmortalLoose() in bytesobject.c, typeobject.c and ceval.c.
Switch more _Py_IsImmortal(...) assertions to _Py_IsImmortalLoose(...) The remaining calls to _Py_IsImmortal are in free-threaded-only code, initialization of core objects, tests, and guards that fall back to code that works with mortal objects. (For this 3.12 backport commit, I re-checked all calls to _Py_IsImmortal; the changed calls are different from the commits to 3.13 & 3.14.)
Crash report
What happened?
While testing
qiskit
, I've found another crash in the stable ABI with 3.13. I've confirmed it with CPython 3.13 as of 3ab8eaf.To reproduce:
Python traceback:
C backtrace:
CC @davidhewitt (this seems to be another problem fixed by PyO3/pyo3#4324)
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Output from running 'python -VV' on the command line:
Python 3.13.0rc1 (main, Aug 16 2024, 17:36:06) [GCC 14.2.0]
Linked PRs
The text was updated successfully, but these errors were encountered: