-
Notifications
You must be signed in to change notification settings - Fork 745
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
Add PyMutex wrappers #4523
Open
ngoldbaum
wants to merge
15
commits into
PyO3:main
Choose a base branch
from
ngoldbaum:pymutex-wrappers
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+178
−9
Open
Add PyMutex wrappers #4523
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
4865289
define public pyo3_ffi::PyMutex::new()
ngoldbaum 874d608
add pyo3::types::mutex for PyMutex wrappers
ngoldbaum b1afb8b
add changelog entry
ngoldbaum b3d193b
update rust API for PyMutex bindings following code review
ngoldbaum 1229634
make the test do something more interesting
ngoldbaum 307e8cf
add Sync impl and make lock accept an immutable reference
ngoldbaum f0ead33
respond to review comments
ngoldbaum 4a0bf3b
add test for PyMutex blocking behavior
ngoldbaum 17263cc
expand docstrings
ngoldbaum 37ffa3a
fix clippy under abi3
ngoldbaum 10f5faf
simplify test
ngoldbaum 7df69a0
use AtomicBool instead of OnceLock<bool>
ngoldbaum e35ea76
use std::hint::spin_loop() per clippy lint
ngoldbaum 604b9bb
make PyMutex::new() const
ngoldbaum 22ee351
fix docstring formatting
ngoldbaum File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
* Added a Rust wrapper for `PyMutex`, available on Python 3.13 and newer. |
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,157 @@ | ||
use std::cell::UnsafeCell; | ||
use std::ops::{Deref, DerefMut}; | ||
|
||
/// Wrapper for [`PyMutex`](https://docs.python.org/3/c-api/init.html#c.PyMutex), exposing an RAII guard interface. | ||
/// | ||
/// Comapred with `std::sync::Mutex` or `parking_lot::Mutex`, this is a very | ||
/// stripped-down locking primitive that only supports blocking lock and unlock | ||
/// operations. | ||
/// | ||
/// `PyMutex` is hooked into CPython's garbage collector and the GIL in GIL-enabled | ||
/// builds. If a thread is blocked on aquiring the mutex and holds the GIL or would | ||
/// prevent Python from entering garbage collection, then Python will release the | ||
/// thread state, allowing garbage collection or other threads blocked by the GIL to | ||
/// proceed. This means it is impossible for PyMutex to deadlock with the GIL. | ||
pub struct PyMutex<T: ?Sized> { | ||
mutex: UnsafeCell<crate::ffi::PyMutex>, | ||
data: UnsafeCell<T>, | ||
} | ||
|
||
/// RAII guard to handle releasing a PyMutex lock. | ||
/// | ||
/// The lock is released when `PyMutexGuard` is dropped. | ||
pub struct PyMutexGuard<'a, T> { | ||
inner: &'a PyMutex<T>, | ||
} | ||
|
||
impl<T> PyMutex<T> { | ||
/// Acquire the mutex, blocking the current thread until it is able to do so. | ||
pub fn lock(&self) -> PyMutexGuard<'_, T> { | ||
unsafe { crate::ffi::PyMutex_Lock(UnsafeCell::raw_get(&self.mutex)) }; | ||
PyMutexGuard { inner: self } | ||
} | ||
|
||
/// Create a new mutex in an unlocked state ready for use. | ||
pub const fn new(value: T) -> Self { | ||
Self { | ||
mutex: UnsafeCell::new(crate::ffi::PyMutex::new()), | ||
data: UnsafeCell::new(value), | ||
} | ||
} | ||
} | ||
|
||
// safety: PyMutex serializes access | ||
unsafe impl<T: Send> Sync for PyMutex<T> {} | ||
|
||
impl<'a, T> Drop for PyMutexGuard<'a, T> { | ||
fn drop(&mut self) { | ||
unsafe { crate::ffi::PyMutex_Unlock(UnsafeCell::raw_get(&self.inner.mutex)) }; | ||
} | ||
} | ||
|
||
impl<'a, T> Deref for PyMutexGuard<'a, T> { | ||
ngoldbaum marked this conversation as resolved.
Show resolved
Hide resolved
|
||
type Target = T; | ||
|
||
fn deref(&self) -> &T { | ||
// safety: cannot be null pointer because PyMutex::new always | ||
// creates a valid PyMutex pointer | ||
unsafe { &*self.inner.data.get() } | ||
} | ||
} | ||
|
||
impl<'a, T> DerefMut for PyMutexGuard<'a, T> { | ||
fn deref_mut(&mut self) -> &mut T { | ||
// safety: cannot be null pointer because PyMutex::new always | ||
// creates a valid PyMutex pointer | ||
unsafe { &mut *self.inner.data.get() } | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use std::sync::{ | ||
atomic::{AtomicBool, Ordering}, | ||
mpsc::sync_channel, | ||
}; | ||
|
||
use super::*; | ||
use crate::types::{PyAnyMethods, PyDict, PyDictMethods, PyNone}; | ||
use crate::Py; | ||
use crate::Python; | ||
|
||
#[test] | ||
fn test_pymutex() { | ||
let mutex = Python::with_gil(|py| -> PyMutex<Py<PyDict>> { | ||
let d = PyDict::new(py); | ||
PyMutex::new(d.unbind()) | ||
}); | ||
|
||
Python::with_gil(|py| { | ||
let mutex = py.allow_threads(|| -> PyMutex<Py<PyDict>> { | ||
std::thread::spawn(|| { | ||
let dict_guard = mutex.lock(); | ||
Python::with_gil(|py| { | ||
let dict = dict_guard.bind(py); | ||
dict.set_item(PyNone::get(py), PyNone::get(py)).unwrap(); | ||
}); | ||
drop(dict_guard); | ||
mutex | ||
}) | ||
.join() | ||
.unwrap() | ||
}); | ||
|
||
let dict_guard = mutex.lock(); | ||
let d = dict_guard.bind(py); | ||
|
||
assert!(d | ||
.get_item(PyNone::get(py)) | ||
.unwrap() | ||
.unwrap() | ||
.eq(PyNone::get(py)) | ||
.unwrap()); | ||
}); | ||
} | ||
|
||
#[test] | ||
fn test_pymutex_blocks() { | ||
let mutex = PyMutex::new(()); | ||
let first_thread_locked_once = AtomicBool::new(false); | ||
let second_thread_locked_once = AtomicBool::new(false); | ||
let finished = AtomicBool::new(false); | ||
let (sender, receiver) = sync_channel::<bool>(0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a |
||
|
||
std::thread::scope(|s| { | ||
s.spawn(|| { | ||
let guard = mutex.lock(); | ||
first_thread_locked_once.store(true, Ordering::SeqCst); | ||
while !finished.load(Ordering::SeqCst) { | ||
if second_thread_locked_once.load(Ordering::SeqCst) { | ||
// Wait a little to guard against the unlikely event that | ||
// the other thread isn't blocked on acquiring the mutex yet. | ||
// If PyMutex had a try_lock implementation this would be | ||
// unnecessary | ||
std::thread::sleep(std::time::Duration::from_millis(10)); | ||
// block (and hold the mutex) until the receiver actually receives something | ||
sender.send(true).unwrap(); | ||
finished.store(true, Ordering::SeqCst); | ||
} | ||
} | ||
drop(guard); | ||
}); | ||
|
||
s.spawn(|| { | ||
while !first_thread_locked_once.load(Ordering::SeqCst) { | ||
std::hint::spin_loop(); | ||
} | ||
second_thread_locked_once.store(true, Ordering::SeqCst); | ||
let guard = mutex.lock(); | ||
assert!(finished.load(Ordering::SeqCst)); | ||
drop(guard); | ||
}); | ||
|
||
// threads are blocked until we receive | ||
receiver.recv().unwrap(); | ||
}); | ||
} | ||
} |
Oops, something went wrong.
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.
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.
Comapred -> Compared