Fix GIL for fft extensions - #3040
Open
abagusetty wants to merge 6 commits into
Open
Conversation
abagusetty
commented
Aug 24, 2026
abagusetty
commented
Contributor
- Have you provided a meaningful PR description?
- Have you added a test, reproducer or referred to an issue with a reproducer?
- Have you tested your changes locally for CPU and GPU devices?
- Have you made sure that new changes do not introduce compiler warnings?
- Have you checked performance impact of proposed changes?
- Have you added documentation for your changes, if necessary?
- Have you added your changes to the changelog?
abagusetty
requested review from
antonwolfy,
ndgrigorian and
vlad-perevezentsev
as code owners
August 24, 2026 14:54
|
Can one of the admins verify this patch? |
Contributor
Author
|
Apologies for not covering this in the earlier PRs. This was caught by an application and resulted in this separate PR |
Contributor
It's absolutely fine. Thank you pushing the fix. |
usm_ndarray::get_data() goes through the Cython api function
UsmNDArray_GetData, which calls into the Python C-API. Calling it after
py::gil_scoped_release aborts the interpreter with
Fatal Python error: PyThreadState_Get: the function must be called
with the GIL held [...] the GIL is released
which crashed every test worker that reached an out-of-place transform.
Read both pointers before the release, matching in_place.tpp.
The GIL test used SyclQueue.wait() as its reference, but by the time it
ran the queue was already drained, so the reference measured no ticks at
all. Compare against a nogil time.sleep() of the same duration instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
|
coverage: 78.482% (+0.005%) from 78.477% — abagusetty:fix/gil-release-fft into IntelPython:master |
Contributor
Author
|
not sure if the CI is stuck or something |
antonwolfy
approved these changes
Aug 25, 2026
antonwolfy
left a comment
antonwolfy
left a comment
Contributor
There was a problem hiding this comment.
Thank you @abagusetty. The change is very helpful.
The only small nit below.
Co-authored-by: Anton <100830759+antonwolfy@users.noreply.github.com>
ndgrigorian
reviewed
Aug 25, 2026
Co-authored-by: ndgrigorian <46709016+ndgrigorian@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.