-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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
Crash in clear_weakref() in pydantic test suite, in 3.12.0rc1 and newer #108295
Comments
Oh, and this is also filed at pydantic/pydantic#7181. I'm not really 100% sure whether pydantic isn't doing something wrong that got exposed here. |
It would be nice if you could provide a pure python code(without pydantic) which crashes =) |
I'm afraid that's as far as I managed (reducing it to one test file). Even with that, there are times when I can't reproduce it a single time — and times when it fails all the time. Right now I'm at the former, so I have no clue how to move forward. |
I've done a few quick tests on a clean system to see what I can find, so far I've found this issue doesn't exist when using 3.12beta4 and below. I'll add an strace from my system as it seems to failing at the same place for me every time and then see if I can confirm anything more to help solve this. |
Eh, this bug really sucks. All I've been able to establish so far is that — obviously — it's GC-related. If I decrease GC threshold low enough or disable GC, it stops happening. If I reduce the file too much, it stops happening. And ofc it also randomly stops happening independently of that, and FWICS it's not affected by So far my only guess is that if something is GC-ed too late, it segfaults. |
I suspect we have to call |
In the meantime, I managed to produce a segfault when allocating a large from typing import TypeVar
from weakref import WeakValueDictionary
vals = WeakValueDictionary()
for x in range(100000):
vals[x] = TypeVar(15*str(x))
print(len(vals))
|
That one doesn't repro for me with a freshly built main:
|
Oh wait, I had already applied my suggested patch when I tried your repro. Without the patch, it does segfault, which strongly suggests that my patch is right. Thanks for the much smaller repro case! The repro from the original post doesn't work on main because pydantic-core doesn't build there. I'll build 3.12 when I get a chance, but in the meantime I'll send a PR for the |
Thanks. I can confirm that the change fixes my reproducer, and probably pydantic too. I've tried 100 iterations of the test call, and it didn't crash (while without the change it crashes on the first attempt). |
That's great to hear! I got the original reproducer running on 3.12, but I get a different crash. Possibly Pydantic just doesn't work with a debug-mode Python?
It's trying to dealloc one of these:
So this seems like a different bug. |
That's possibly related to PyO3/pyo3#3064. In general I think it's a reasonable objective for us to support debug builds, but we can't easily test them in CI (actions/setup-python#86) |
|
Ideally all C extensions should work in a pydebug build, but that's a bigger project and it sounds like it would also need work in PyO3. |
(cherry picked from commit 482fad7) Co-authored-by: Jelle Zijlstra <[email protected]>
It looks to me that this has been resolved. |
Thanks, yes I will investigate debug build support in pyo3/ pydantic separately. |
Crash report
CPython versions tested on:
3.12, CPython main branch
Operating systems tested on:
Linux
Output from running 'python -VV' on the command line:
Python 3.12.0rc1+ (heads/3.12:149d70c254, Aug 22 2023, 16:11:47) [GCC 13.2.0]
What happened?
The pydantic test suite started causing pytest to segfault on exit recently, with Python 3.12. I've been able to bisect it to pydantic commit pydantic/pydantic@f5e3aa9 that looks relatively harmless. I've also been able to bisect CPython into commit 58f9c88:
I have been able to reproduce the problem with tip of
main
as well, though I have to note it's a bit of heisenbug. I've basically had a very lucky day today that I've managed to correctly bisect on the second attempt.To reproduce:
Error messages
Backtrace
Linked PRs
The text was updated successfully, but these errors were encountered: