# HG changeset patch # User Yuya Nishihara # Date 1567327050 -32400 # 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 -r 01d3ce3281cf -r 8db8fa1de2ef rust/hg-cpython/src/dirstate/dirs_multiset.rs --- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs Sun Sep 01 17:35:14 2019 +0900 +++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs Sun Sep 01 17:37:30 2019 +0900 @@ -16,11 +16,12 @@ 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 @@ Self::create_instance( py, - RefCell::new(inner), + PySharedRefCell::new(inner), PySharedState::default() ) } @@ -104,7 +105,11 @@ 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 -r 01d3ce3281cf -r 8db8fa1de2ef rust/hg-cpython/src/dirstate/dirstate_map.rs --- a/rust/hg-cpython/src/dirstate/dirstate_map.rs Sun Sep 01 17:35:14 2019 +0900 +++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs Sun Sep 01 17:37:30 2019 +0900 @@ -21,7 +21,7 @@ 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 @@ // 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 -r 01d3ce3281cf -r 8db8fa1de2ef rust/hg-cpython/src/ref_sharing.rs --- a/rust/hg-cpython/src/ref_sharing.rs Sun Sep 01 17:35:14 2019 +0900 +++ b/rust/hg-cpython/src/ref_sharing.rs Sun Sep 01 17:37:30 2019 +0900 @@ -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 @@ 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 @@ } } +/// `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 @@ /// } /// /// py_class!(pub class MyType |py| { -/// data inner: RefCell; +/// data inner: PySharedRefCell; /// data py_shared_state: PySharedState; /// }); /// @@ -177,16 +209,21 @@ 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 @@ /// } /// /// py_class!(pub class MyType |py| { -/// data inner: RefCell; +/// data inner: PySharedRefCell; /// data py_shared_state: PySharedState; /// /// def __iter__(&self) -> PyResult {