-
Notifications
You must be signed in to change notification settings - Fork 112
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
support 3.13t free-threaded python #471
base: main
Are you sure you want to change the base?
Conversation
// FIXME probably a deadlock risk here due to the GIL? Might need MutexExt trait in PyO3 | ||
let mut dtypes = self.dtypes.lock().expect("dtype cache poisoned"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choice to use Mutex
here makes me want to add a MutexExt
trait to PyO3 similar to OnceLockExt
which we already added.
Given that writes to this should be infrequent (it's a global cache, as far as I can tell), I also wonder if RwLock
is appropriate here. Readers are extremely short-lived so the analysis in https://blog.nelhage.com/post/rwlock-contention/ seems to be a non-issue (cc @alex)
... in which case I want RwLockExt
too 😂
Based on the failures here, I propose rust-numpy 0.23.0 releases without free-threading support, and we seek to follow up in 0.23.1. |
I did a bit of investigating here. I believe the fundamental problem is that the borrow checking API relies on the GIL for synchronization of exclusive access rust-numpy/src/borrow/shared.rs Lines 45 to 48 in b34bc29
On the free-threaded build this is not true anymore, and therefore unsound. An easy (but probably not optimal) solution is to wrap |
First pass at adding CI and tests for freethreaded Python.
Locally I get some test failures, so I will have to investigate further before this is ready even if CI is green. cc @ngoldbaum