Skip to content
Snippets Groups Projects
Commit d1e304025b90 authored by Georges Racinet's avatar Georges Racinet :squid:
Browse files

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.
parent d85514a88706
No related branches found
No related tags found
2 merge requests!1292Draft: 7.0rc preparation,!1202Add a PyO3 version of `PySharedRef`
......@@ -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,7 +230,7 @@
}
impl<'py, T: ?Sized> PySharedRef<'py, T> {
/// Immutably borrows the wrapped value.
/// Take the lock on the wrapped value for read-only operations.
///
/// # Panics
///
......@@ -234,10 +234,10 @@
///
/// # 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.
......@@ -240,6 +240,6 @@
}
/// 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
......@@ -245,6 +245,6 @@
// 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()
}
......@@ -248,9 +248,9 @@
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
///
......@@ -252,12 +252,12 @@
///
/// 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.
......@@ -260,8 +260,8 @@
}
/// 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,6 +291,6 @@
) -> 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,
......@@ -295,6 +295,6 @@
// 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
......
......@@ -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,6 +119,6 @@
///
/// 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) };
......@@ -123,5 +123,5 @@
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());
}
......@@ -126,5 +126,5 @@
}
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) };
......@@ -129,5 +129,5 @@
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());
}
......@@ -132,5 +132,5 @@
}
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) };
......@@ -135,6 +135,6 @@
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]
......@@ -138,5 +138,5 @@
}
#[test]
fn test_try_borrow_mut_while_leaked_ref() -> PyResult<()> {
fn test_try_write_while_leaked_ref() -> PyResult<()> {
with_setup(|py, owner| {
......@@ -142,5 +142,5 @@
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();
......@@ -144,6 +144,6 @@
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();
......@@ -148,4 +148,4 @@
{
let _leaked_ref2 = unsafe { leaked.try_borrow(py) }.unwrap();
assert_try_borrow_string_mut_err(owner);
assert_try_write_string_err(owner);
}
......@@ -151,3 +151,3 @@
}
assert_try_borrow_string_mut_err(owner);
assert_try_write_string_err(owner);
}
......@@ -153,7 +153,7 @@
}
assert_try_borrow_string_mut_ok(owner);
assert_try_write_string_ok(owner);
Ok(())
})
}
#[test]
......@@ -155,7 +155,7 @@
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| {
......@@ -161,7 +161,7 @@
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();
......@@ -163,7 +163,7 @@
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);
}
......@@ -169,7 +169,7 @@
}
assert_try_borrow_string_mut_ok(owner);
assert_try_write_string_ok(owner);
Ok(())
})
}
#[test]
......@@ -171,9 +171,9 @@
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) };
......@@ -177,7 +177,7 @@
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,7 +186,7 @@
#[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) };
......@@ -190,7 +190,7 @@
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,7 +199,7 @@
}
#[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) };
......@@ -203,5 +203,5 @@
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();
......@@ -207,8 +207,8 @@
assert_try_borrow_string_mut_err(owner);
assert_try_write_string_err(owner);
Ok(())
})
}
#[test]
#[should_panic(expected = "already borrowed")]
......@@ -209,10 +209,10 @@
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) };
......@@ -216,6 +216,6 @@
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) };
......@@ -220,9 +220,9 @@
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]
......@@ -223,10 +223,10 @@
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) };
......@@ -230,5 +230,5 @@
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();
......@@ -234,8 +234,8 @@
assert_try_borrow_string_err(owner);
assert_try_read_string_err(owner);
Ok(())
})
}
#[test]
#[should_panic(expected = "already mutably borrowed")]
......@@ -236,10 +236,10 @@
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) };
......@@ -243,6 +243,6 @@
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) };
......@@ -247,6 +247,6 @@
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")
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment