Mercurial > public > mercurial-scm > hg
changeset 52609:d1e304025b90
rust-pyo3-sharedref: replaced borrow/borrow_mut with RwLock namings
The "borrow" word had way too many uses in this context. Originally,
it came from the rust-cpython version, which is based on `RefCell`.
Before this change, we had untracktable stacks of borrows (first the
`Bound`, then the `PySharedRefCell`).
In any case, the change to `RwLock` is far from being neutral, and
we may need in a future without GIL to also expose blocking methods, to
account for in-Rust possible concurrency. Using `read()` and `write()`
right now matches our PyO3 habits anyway, where all non-Sync objects
have to be wrapped in a RwLock.
author | Georges Racinet <georges.racinet@cloudcrane.io> |
---|---|
date | Sun, 15 Dec 2024 13:58:31 +0100 |
parents | d85514a88706 |
children | c25d345f5aa5 |
files | rust/pyo3-sharedref/src/lib.rs rust/pyo3-sharedref/tests/test_sharedref.rs |
diffstat | 2 files changed, 52 insertions(+), 52 deletions(-) [+] |
line wrap: on
line diff
--- a/rust/pyo3-sharedref/src/lib.rs Sun Dec 15 14:42:53 2024 +0100 +++ b/rust/pyo3-sharedref/src/lib.rs Sun Dec 15 13:58:31 2024 +0100 @@ -93,7 +93,7 @@ /// fn add(slf: &Bound<'_, Self>, i: i32) -> PyResult<()> { /// let rust_set = &slf.borrow().rust_set; /// let shared_ref = unsafe { rust_set.borrow_with_owner(slf) }; -/// let mut set_ref = shared_ref.borrow_mut(); +/// let mut set_ref = shared_ref.write(); /// set_ref.insert(i); /// Ok(()) /// } @@ -230,38 +230,38 @@ } impl<'py, T: ?Sized> PySharedRef<'py, T> { - /// Immutably borrows the wrapped value. + /// Take the lock on the wrapped value for read-only operations. /// /// # Panics /// - /// Panics if the value is currently mutably borrowed. - pub fn borrow(&self) -> RwLockReadGuard<'py, T> { - self.try_borrow().expect("already mutably borrowed") + /// Panics if the lock is currently held for write operations. + pub fn read(&self) -> RwLockReadGuard<'py, T> { + self.try_read().expect("already mutably borrowed") } /// Immutably borrows the wrapped value, returning an error if the value /// is currently mutably borrowed. - pub fn try_borrow(&self) -> TryLockResult<RwLockReadGuard<'py, T>> { + pub fn try_read(&self) -> TryLockResult<RwLockReadGuard<'py, T>> { // state isn't involved since - // - data.try_borrow() would fail if self is mutably borrowed, - // - and data.try_borrow_mut() would fail while self is borrowed. + // - data.try_read() would fail if self is mutably borrowed, + // - and data.try_write() would fail while self is borrowed. self.data.try_read() } - /// Mutably borrows the wrapped value. + /// Take the lock on the wrapped value for write operations. /// /// Any existing leaked references will be invalidated. /// /// # Panics /// - /// Panics if the value is currently borrowed. - pub fn borrow_mut(&self) -> RwLockWriteGuard<'py, T> { - self.try_borrow_mut().expect("already borrowed") + /// Panics if the lock is currently held. + pub fn write(&self) -> RwLockWriteGuard<'py, T> { + self.try_write().expect("already borrowed") } /// Mutably borrows the wrapped value, returning an error if the value /// is currently borrowed. - pub fn try_borrow_mut(&self) -> TryLockResult<RwLockWriteGuard<'py, T>> { + pub fn try_write(&self) -> TryLockResult<RwLockWriteGuard<'py, T>> { // the value may be immutably borrowed through UnsafePyLeaked if self.state.current_borrow_count(self.py()) > 0 { // propagate borrow-by-leaked state to data to get BorrowMutError @@ -291,10 +291,10 @@ ) -> Result<UnsafePyLeaked<&'static T>, TryLeakError> { // make sure self.data isn't mutably borrowed; otherwise the // generation number wouldn't be trusted. - let data_ref = self.try_borrow()?; + let data_ref = self.try_read()?; // keep reference to the owner so the data and state are alive, - // but the data pointer can be invalidated by borrow_mut(). + // but the data pointer can be invalidated by write(). // the state wouldn't since it is immutable. let state_ptr: *const PySharedState = self.state; let data_ptr: *const T = &*data_ref; @@ -322,7 +322,7 @@ /// as follows: /// /// - The immutability of `PycCass` object fields. Any mutation of -/// [`PySharedRefCell`] is allowed only through its `borrow_mut()`. +/// [`PySharedRefCell`] is allowed only through its `write()`. /// - The `py: Python<'_>` token, which makes sure that any data access is /// synchronized by the GIL. /// - The underlying `RefCell`, which prevents `PySharedRefCell` value from @@ -330,9 +330,9 @@ /// - The `borrow_count`, which is the number of references borrowed from /// `UnsafePyLeaked`. Just like `RefCell`, mutation is prohibited while /// `UnsafePyLeaked` is borrowed. -/// - The `generation` counter, which increments on `borrow_mut()`. -/// `UnsafePyLeaked` reference is valid only if the `current_generation()` -/// equals to the `generation` at the time of `leak_immutable()`. +/// - The `generation` counter, which increments on `write()`. `UnsafePyLeaked` +/// reference is valid only if the `current_generation()` equals to the +/// `generation` at the time of `leak_immutable()`. #[derive(Debug)] struct PySharedState { // The counter variable could be Cell<usize> since any operation on
--- a/rust/pyo3-sharedref/tests/test_sharedref.rs Sun Dec 15 14:42:53 2024 +0100 +++ b/rust/pyo3-sharedref/tests/test_sharedref.rs Sun Dec 15 13:58:31 2024 +0100 @@ -53,7 +53,7 @@ ) -> () { let cell = &owner.borrow_mut().string; let shared_ref = unsafe { cell.borrow_with_owner(owner) }; - f(&mut shared_ref.borrow_mut()); + f(&mut shared_ref.write()); } #[test] @@ -119,65 +119,65 @@ /// /// Simply returning the `Result` is not possible, because that is /// returning a reference to data owned by the function -fn assert_try_borrow_string_mut_ok(owner: &Bound<'_, Owner>) { +fn assert_try_write_string_ok(owner: &Bound<'_, Owner>) { let cell = &owner.borrow().string; let shared_ref = unsafe { cell.borrow_with_owner(owner) }; - assert!(shared_ref.try_borrow_mut().is_ok()); + assert!(shared_ref.try_write().is_ok()); } -fn assert_try_borrow_string_mut_err(owner: &Bound<'_, Owner>) { +fn assert_try_write_string_err(owner: &Bound<'_, Owner>) { let cell = &owner.borrow().string; let shared_ref = unsafe { cell.borrow_with_owner(owner) }; - assert!(shared_ref.try_borrow_mut().is_err()); + assert!(shared_ref.try_write().is_err()); } -fn assert_try_borrow_string_err(owner: &Bound<'_, Owner>) { +fn assert_try_read_string_err(owner: &Bound<'_, Owner>) { let cell = &owner.borrow().string; let shared_ref = unsafe { cell.borrow_with_owner(owner) }; - assert!(shared_ref.try_borrow().is_err()); + assert!(shared_ref.try_read().is_err()); } #[test] -fn test_try_borrow_mut_while_leaked_ref() -> PyResult<()> { +fn test_try_write_while_leaked_ref() -> PyResult<()> { with_setup(|py, owner| { - assert_try_borrow_string_mut_ok(owner); + assert_try_write_string_ok(owner); let leaked = leak_string(owner); { let _leaked_ref = unsafe { leaked.try_borrow(py) }.unwrap(); - assert_try_borrow_string_mut_err(owner); + assert_try_write_string_err(owner); { let _leaked_ref2 = unsafe { leaked.try_borrow(py) }.unwrap(); - assert_try_borrow_string_mut_err(owner); + assert_try_write_string_err(owner); } - assert_try_borrow_string_mut_err(owner); + assert_try_write_string_err(owner); } - assert_try_borrow_string_mut_ok(owner); + assert_try_write_string_ok(owner); Ok(()) }) } #[test] -fn test_try_borrow_mut_while_leaked_ref_mut() -> PyResult<()> { +fn test_try_write_while_leaked_ref_mut() -> PyResult<()> { with_setup(|py, owner| { - assert_try_borrow_string_mut_ok(owner); + assert_try_write_string_ok(owner); let leaked = leak_string(owner); let mut leaked_iter = unsafe { leaked.map(py, |s| s.chars()) }; { let _leaked_ref = unsafe { leaked_iter.try_borrow_mut(py) }.unwrap(); - assert_try_borrow_string_mut_err(owner); + assert_try_write_string_err(owner); } - assert_try_borrow_string_mut_ok(owner); + assert_try_write_string_ok(owner); Ok(()) }) } #[test] -fn test_try_leak_while_borrow_mut() -> PyResult<()> { +fn test_try_leak_while_write() -> PyResult<()> { with_setup(|_py, owner| { let cell = &owner.borrow().string; let shared_ref = unsafe { cell.borrow_with_owner(owner) }; - let _mut_ref = shared_ref.borrow_mut(); + let _mut_ref = shared_ref.write(); assert!(try_leak_string(owner).is_err()); Ok(()) @@ -186,11 +186,11 @@ #[test] #[should_panic(expected = "already mutably borrowed")] -fn test_leak_while_borrow_mut() { +fn test_leak_while_write() { with_setup(|_py, owner| { let cell = &owner.borrow().string; let shared_ref = unsafe { cell.borrow_with_owner(owner) }; - let _mut_ref = shared_ref.borrow_mut(); + let _mut_ref = shared_ref.write(); leak_string(owner); Ok(()) @@ -199,54 +199,54 @@ } #[test] -fn test_try_borrow_mut_while_borrow() -> PyResult<()> { +fn test_try_write_while_borrow() -> PyResult<()> { with_setup(|_py, owner| { let cell = &owner.borrow().string; let shared_ref = unsafe { cell.borrow_with_owner(owner) }; - let _ref = shared_ref.borrow(); + let _ref = shared_ref.read(); - assert_try_borrow_string_mut_err(owner); + assert_try_write_string_err(owner); Ok(()) }) } #[test] #[should_panic(expected = "already borrowed")] -fn test_borrow_mut_while_borrow() { +fn test_write_while_borrow() { with_setup(|_py, owner| { let cell = &owner.borrow().string; let shared_ref = unsafe { cell.borrow_with_owner(owner) }; - let _ref = shared_ref.borrow(); + let _ref = shared_ref.read(); let shared_ref2 = unsafe { cell.borrow_with_owner(owner) }; - let _mut_ref = shared_ref2.borrow_mut(); + let _mut_ref = shared_ref2.write(); Ok(()) }) .expect("should already have panicked") } #[test] -fn test_try_borrow_while_borrow_mut() -> PyResult<()> { +fn test_try_borrow_while_write() -> PyResult<()> { with_setup(|_py, owner| { let cell = &owner.borrow().string; let shared_ref = unsafe { cell.borrow_with_owner(owner) }; - let _mut_ref = shared_ref.borrow_mut(); + let _mut_ref = shared_ref.write(); - assert_try_borrow_string_err(owner); + assert_try_read_string_err(owner); Ok(()) }) } #[test] #[should_panic(expected = "already mutably borrowed")] -fn test_borrow_while_borrow_mut() { +fn test_borrow_while_write() { with_setup(|_py, owner| { let cell = &owner.borrow().string; let shared_ref = unsafe { cell.borrow_with_owner(owner) }; - let _mut_ref = shared_ref.borrow_mut(); + let _mut_ref = shared_ref.write(); let shared_ref2 = unsafe { cell.borrow_with_owner(owner) }; - let _ref = shared_ref2.borrow(); + let _ref = shared_ref2.read(); Ok(()) }) .expect("should already have panicked")