rust-cpython: leverage RefCell::borrow() to guarantee there's no mutable ref
Since the underlying value can't be mutably borrowed by PyLeaked, we don't
have to manage yet another mutably-borrowed state. We can just rely on the
RefCell implementation.
Maybe we can add try_leak_immutable(), but this patch doesn't in order to
keep the patch series not too long.
--- a/rust/hg-cpython/src/ref_sharing.rs Sat Oct 12 20:48:30 2019 +0900
+++ b/rust/hg-cpython/src/ref_sharing.rs Sat Oct 05 08:56:15 2019 -0400
@@ -38,6 +38,8 @@
/// `PySharedRefCell` is allowed only through its `borrow_mut()`.
/// - The `py: Python<'_>` token, which makes sure that any data access is
/// synchronized by the GIL.
+/// - The underlying `RefCell`, which prevents `PySharedRefCell` data from
+/// being directly borrowed or leaked while it is mutably borrowed.
/// - The `borrow_count`, which is the number of references borrowed from
/// `PyLeaked`. Just like `RefCell`, mutation is prohibited while `PyLeaked`
/// is borrowed.
@@ -99,7 +101,7 @@
unsafe fn leak_immutable<T>(
&self,
py: Python,
- data: &PySharedRefCell<T>,
+ data: Ref<T>,
) -> PyResult<(&'static T, &'static PySharedState)> {
if self.mutably_borrowed.get() {
return Err(AlreadyBorrowed::new(
@@ -108,10 +110,8 @@
mutable reference in Python objects",
));
}
- // TODO: it's weird that self is data.py_shared_state. Maybe we
- // can move stuff to PySharedRefCell?
- let ptr = data.as_ptr();
- let state_ptr: *const PySharedState = &data.py_shared_state;
+ let ptr: *const T = &*data;
+ let state_ptr: *const PySharedState = self;
Ok((&*ptr, &*state_ptr))
}
@@ -200,10 +200,6 @@
self.inner.borrow()
}
- fn as_ptr(&self) -> *mut T {
- self.inner.as_ptr()
- }
-
// TODO: maybe this should be named as try_borrow_mut(), and use
// inner.try_borrow_mut(). The current implementation panics if
// self.inner has been borrowed, but returns error if py_shared_state
@@ -242,11 +238,18 @@
}
/// Returns a leaked reference.
+ ///
+ /// # Panics
+ ///
+ /// Panics if this is mutably borrowed.
pub fn leak_immutable(&self) -> PyResult<PyLeaked<&'static T>> {
let state = &self.data.py_shared_state;
+ // make sure self.data isn't mutably borrowed; otherwise the
+ // generation number can't be trusted.
+ let data_ref = self.borrow();
unsafe {
let (static_ref, static_state_ref) =
- state.leak_immutable(self.py, self.data)?;
+ state.leak_immutable(self.py, data_ref)?;
Ok(PyLeaked::new(
self.py,
self.owner,
@@ -702,4 +705,13 @@
}
assert!(owner.string_shared(py).borrow_mut().is_ok());
}
+
+ #[test]
+ #[should_panic(expected = "mutably borrowed")]
+ fn test_leak_while_borrow_mut() {
+ let (gil, owner) = prepare_env();
+ let py = gil.python();
+ let _mut_ref = owner.string_shared(py).borrow_mut();
+ let _ = owner.string_shared(py).leak_immutable();
+ }
}