changeset 42855:8db8fa1de2ef

rust-cpython: introduce restricted variant of RefCell

This should catch invalid borrow_mut() calls. Still the ref-sharing
interface is unsafe.

--- 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<DirsMultiset>;
+    data inner: PySharedRefCell<DirsMultiset>;
     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> {
-        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<u8>) -> PyResult<Option<PyBytes>> {

--- 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<RustDirstateMap>;
+    data inner: PySharedRefCell<RustDirstateMap>;
     data py_shared_state: PySharedState;
 
     def __new__(_cls, _root: PyObject) -> PyResult<Self> {
         let inner = RustDirstateMap::default();
         Self::create_instance(
             py,
-            RefCell::new(inner),
+            PySharedRefCell::new(inner),
             PySharedState::default()
         )
     }

--- 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<T>(
         &self,
         py: Python,
-        data: &RefCell<T>,
+        data: &PySharedRefCell<T>,
     ) -> 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<T> {
+    inner: RefCell<T>,
+}
+
+impl<T> PySharedRefCell<T> {
+    pub const fn new(value: T) -> PySharedRefCell<T> {
+        Self {
+            inner: RefCell::new(value),
+        }
+    }
+
+    pub fn borrow(&self) -> Ref<T> {
+        // 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<T> {
+        // 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<MyStruct>;
+///     data inner: PySharedRefCell<MyStruct>;
 ///     data py_shared_state: PySharedState;
 /// });
 ///
@@ -177,16 +209,21 @@
                 py: Python<'a>,
             ) -> PyResult<crate::ref_sharing::PyRefMut<'a, $inner_struct>>
             {
+                // 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<MyStruct>;
+///     data inner: PySharedRefCell<MyStruct>;
 ///     data py_shared_state: PySharedState;
 ///
 ///     def __iter__(&self) -> PyResult<MyTypeItemsIterator> {