# HG changeset patch # User Yuya Nishihara # Date 2019-09-01 08:37:30 # Node ID 8db8fa1de2ef7ff18a60fd9ed889d990e7907cec # Parent 01d3ce3281cf68fdfa6e63d47093dff78da46903 rust-cpython: introduce restricted variant of RefCell This should catch invalid borrow_mut() calls. Still the ref-sharing interface is unsafe. diff --git a/rust/hg-cpython/src/dirstate/dirs_multiset.rs b/rust/hg-cpython/src/dirstate/dirs_multiset.rs --- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs +++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs @@ -16,11 +16,12 @@ use cpython::{ Python, }; -use crate::{dirstate::extract_dirstate, ref_sharing::PySharedState}; +use crate::dirstate::extract_dirstate; +use crate::ref_sharing::{PySharedRefCell, PySharedState}; use hg::{DirsMultiset, DirstateMapError, DirstateParseError, EntryState}; py_class!(pub class Dirs |py| { - data inner: RefCell; + data inner: PySharedRefCell; data py_shared_state: PySharedState; // `map` is either a `dict` or a flat iterator (usually a `set`, sometimes @@ -53,7 +54,7 @@ py_class!(pub class Dirs |py| { Self::create_instance( py, - RefCell::new(inner), + PySharedRefCell::new(inner), PySharedState::default() ) } @@ -104,7 +105,11 @@ py_shared_ref!(Dirs, DirsMultiset, inner impl Dirs { pub fn from_inner(py: Python, d: DirsMultiset) -> PyResult { - Self::create_instance(py, RefCell::new(d), PySharedState::default()) + Self::create_instance( + py, + PySharedRefCell::new(d), + PySharedState::default(), + ) } fn translate_key(py: Python, res: &Vec) -> PyResult> { diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs --- a/rust/hg-cpython/src/dirstate/dirstate_map.rs +++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs @@ -21,7 +21,7 @@ use libc::c_char; use crate::{ dirstate::copymap::{CopyMap, CopyMapItemsIterator, CopyMapKeysIterator}, dirstate::{decapsule_make_dirstate_tuple, dirs_multiset::Dirs}, - ref_sharing::PySharedState, + ref_sharing::{PySharedRefCell, PySharedState}, }; use hg::{ DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap, @@ -41,14 +41,14 @@ use hg::{ // All attributes also have to have a separate refcount data attribute for // leaks, with all methods that go along for reference sharing. py_class!(pub class DirstateMap |py| { - data inner: RefCell; + data inner: PySharedRefCell; data py_shared_state: PySharedState; def __new__(_cls, _root: PyObject) -> PyResult { let inner = RustDirstateMap::default(); Self::create_instance( py, - RefCell::new(inner), + PySharedRefCell::new(inner), PySharedState::default() ) } diff --git a/rust/hg-cpython/src/ref_sharing.rs b/rust/hg-cpython/src/ref_sharing.rs --- a/rust/hg-cpython/src/ref_sharing.rs +++ b/rust/hg-cpython/src/ref_sharing.rs @@ -9,7 +9,7 @@ use crate::exceptions::AlreadyBorrowed; use cpython::{PyResult, Python}; -use std::cell::{Cell, RefCell, RefMut}; +use std::cell::{Cell, Ref, RefCell, RefMut}; /// Manages the shared state between Python and Rust #[derive(Default)] @@ -61,7 +61,7 @@ impl PySharedState { pub fn leak_immutable( &self, py: Python, - data: &RefCell, + data: &PySharedRefCell, ) -> PyResult<&'static T> { if self.mutably_borrowed.get() { return Err(AlreadyBorrowed::new( @@ -84,6 +84,38 @@ impl PySharedState { } } +/// `RefCell` wrapper to be safely used in conjunction with `PySharedState`. +/// +/// Only immutable operation is allowed through this interface. +#[derive(Debug)] +pub struct PySharedRefCell { + inner: RefCell, +} + +impl PySharedRefCell { + pub const fn new(value: T) -> PySharedRefCell { + Self { + inner: RefCell::new(value), + } + } + + pub fn borrow(&self) -> Ref { + // py_shared_state isn't involved since + // - inner.borrow() would fail if self is mutably borrowed, + // - and inner.borrow_mut() would fail while self is borrowed. + self.inner.borrow() + } + + pub fn as_ptr(&self) -> *mut T { + self.inner.as_ptr() + } + + pub unsafe fn borrow_mut(&self) -> RefMut { + // must be borrowed by self.py_shared_state(py).borrow_mut(). + self.inner.borrow_mut() + } +} + /// Holds a mutable reference to data shared between Python and Rust. pub struct PyRefMut<'a, T> { inner: RefMut<'a, T>, @@ -158,7 +190,7 @@ impl<'a, T> Drop for PyRefMut<'a, T> { /// } /// /// py_class!(pub class MyType |py| { -/// data inner: RefCell; +/// data inner: PySharedRefCell; /// data py_shared_state: PySharedState; /// }); /// @@ -177,16 +209,21 @@ macro_rules! py_shared_ref { py: Python<'a>, ) -> PyResult> { + // assert $data_member type + use crate::ref_sharing::PySharedRefCell; + let data: &PySharedRefCell<_> = self.$data_member(py); self.py_shared_state(py) - .borrow_mut(py, self.$data_member(py).borrow_mut()) + .borrow_mut(py, unsafe { data.borrow_mut() }) } fn leak_immutable<'a>( &'a self, py: Python<'a>, ) -> PyResult<&'static $inner_struct> { - self.py_shared_state(py) - .leak_immutable(py, self.$data_member(py)) + // assert $data_member type + use crate::ref_sharing::PySharedRefCell; + let data: &PySharedRefCell<_> = self.$data_member(py); + self.py_shared_state(py).leak_immutable(py, data) } } @@ -295,7 +332,7 @@ macro_rules! py_shared_iterator_impl { /// } /// /// py_class!(pub class MyType |py| { -/// data inner: RefCell; +/// data inner: PySharedRefCell; /// data py_shared_state: PySharedState; /// /// def __iter__(&self) -> PyResult {