8000 BUG: avoid deadlocks with C++ shared mutex in dispatch cache by ngoldbaum · Pull Request #28577 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

BUG: avoid deadlocks with C++ shared mutex in dispatch cache #28577

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 24, 2025
Merged

BUG: avoid deadlocks with C++ shared mutex in dispatch cache #28577

merged 1 commit into from
Mar 24, 2025

Conversation

ngoldbaum
Copy link
Member

@pitrou correctly pointed out here that the C++ shared_mutex we use in the dispatch might deadlock with the GIL (if it's re-enabled) or other global synchronization events in the interpreter, and we need to explicitly call Py_BEGIN_ALLOW_THREADS/Py_END_ALLOW_THREADS around a possibly blocking call.

I don't have a case where a deadlock happens right now, so no new test. Happy to add one if someone can come up with a way to trigger the deadlock.

I ran the test script from #27786 and don't see any performance hit.

@seberg
Copy link
Member
seberg commented Mar 24, 2025

I ran the test script from #27786 and don't see any performance hit.

I suppose that is when the GIL isn't used, but then I guess free-threading + GIL used isn't something we have to worry about :).

seberg
seberg approved these changes Mar 24, 2025
View reviewed changes
Copy link
Member
@seberg seberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks, please feel free to merge once tests pass.

@ngoldbaum
Copy link
Member Author

but then I guess free-threading + GIL used isn't something we have to worry about :)

not from a multithreaded performance perspective anyway

@seberg
Copy link
Member
seberg commented Mar 24, 2025

Grabbing the GIL used to be slowish, so I thought you might see it in the timings. But it may be much faster now (if there is no contention at least).

@ngoldbaum ngoldbaum added the 09 - Backport-Candidate PRs tagged should be backported label Mar 24, 2025
@ngoldbaum
Copy link
Member Author

I think this is safe to backport.

@ngoldbaum ngoldbaum merged commit 342ffc4 into numpy:main Mar 24, 2025
73 checks passed
@ngoldbaum ngoldbaum added the 39 - free-threading PRs and issues related to support for free-threading CPython (a.k.a. no-GIL, PEP 703) label Mar 24, 2025
@charris charris mentioned this pull request Mar 24, 2025
Merged
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seberg seberg seberg approved these changes

Assignees
No one assigned
Labels
00 - Bug 39 - free-threading PRs and issues related to support for free-threading CPython (a.k.a. no-GIL, PEP 703)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ngoldbaum @seberg @charris
0