-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[Python] SEGFAULT in test_udf_via_substrait when run under CPython debug build #43487
Comments
@lysnikolaou thanks for the report and analysis! I am not super familiar with this part of our code base, but from reading your analysis, is it that we should ensure the GIL is held when calling that cc @pitrou |
That's correct, yes! |
I'm taking a look at this. |
…plementation 1. Remove spurious increfs (the function object is already incref'ed at an upper level) 2. Add unit test with an ephemeral Python function object 3. Streamline and improve Python reference handling
…reter (#43565) ### Rationale for this change Debug builds of CPython help catch low-level errors when using the Python C API. This is illustrated in GH-43487: a debug build of CPython detected that we were incref'ing a Python object without holding the GIL (which is a race condition otherwise). ### What changes are included in this PR? 1. Add a Docker build with a conda-installed debug interpreter. 2. Add a Crossbow job to run said Docker build with Python 3.12. ### Are these changes tested? Yes, by the adding Crossbow job. The job now fails with a crash in `test_udf.py`, because of GH-43487. ### Are there any user-facing changes? No. * GitHub Issue: #43559 Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: Antoine Pitrou <antoine@python.org>
…plementation 1. Remove spurious increfs (the function object is already incref'ed at an upper level) 2. Add unit test with an ephemeral Python function object 3. Streamline and improve Python reference handling
…tation (#43557) 1. Remove spurious increfs (the function object is already incref'ed at an upper level) 2. Add unit test with an ephemeral Python function object 3. Streamline and improve Python reference handling * GitHub Issue: #43487 Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: Antoine Pitrou <antoine@python.org>
Issue resolved by pull request 43557 |
Thanks @pitrou! |
Thanks for your report @lysnikolaou , this helped us improve PyArrow CI and quality. |
Describe the bug, including details regarding any error messages, version, and platform.
Hey everyone! 👋
I'm trying to build arrow and PyArrow with a debug build of CPython and run the test suite, but I keep running into a segmentation fault. The crash happens in
test_udf_via_substrait
here, and I can reproduce it using both 3.13.0b4+ and 3.12.4.The source of the segmentation fault is the
Py_INCREF
that's happening here. Because this is under a debug build,Py_INCREF
tries to access the thread state to increase the aggregate reference count.Stripped stack trace to `Py_INCREF` call
However, this is all happening in a
with nogil
Cython branch, which means that the thread state isNULL
. The last GIL acquire/release cycle before the crash is happening due to thisPyAcquireGIL
.Stripped stack trace to setting the thread state to `NULL`
Component(s)
Python
The text was updated successfully, but these errors were encountered: