rust-cpython: require GIL to borrow immutable reference from PySharedRefCell

Since the inner value may be leaked, we probably need GIL to guarantee that
there's no data race.

inner(py).borrow() is replaced with inner_shared(py).borrow(), which basically
means any PySharedRefCell data should be accessed through PySharedRef wrapper.

rust/hg-cpython/src/dirstate/dirs_multiset.rs

@@ -1,129 +1,129 b''
 // dirs_multiset.rs
 //
 // Copyright 2019 Raphaël Gomès <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 //! Bindings for the `hg::dirstate::dirs_multiset` file provided by the
 //! `hg-core` package.
 
 use std::cell::RefCell;
 use std::convert::TryInto;
 
 use cpython::{
     exc, ObjectProtocol, PyBytes, PyClone, PyDict, PyErr, PyObject, PyResult,
     Python,
 };
 
 use crate::dirstate::extract_dirstate;
 use crate::ref_sharing::{PyLeakedRef, PySharedRefCell};
 use hg::{
     utils::hg_path::{HgPath, HgPathBuf},
     DirsMultiset, DirsMultisetIter, DirstateMapError, DirstateParseError,
     EntryState,
 };
 
 py_class!(pub class Dirs |py| {
     data inner: PySharedRefCell<DirsMultiset>;
 
     // `map` is either a `dict` or a flat iterator (usually a `set`, sometimes
     // a `list`)
     def __new__(
         _cls,
         map: PyObject,
         skip: Option<PyObject> = None
     ) -> PyResult<Self> {
         let mut skip_state: Option<EntryState> = None;
         if let Some(skip) = skip {
             skip_state = Some(
                 skip.extract::<PyBytes>(py)?.data(py)[0]
                     .try_into()
                     .map_err(|e: DirstateParseError| {
                         PyErr::new::<exc::ValueError, _>(py, e.to_string())
                     })?,
             );
         }
         let inner = if let Ok(map) = map.cast_as::<PyDict>(py) {
             let dirstate = extract_dirstate(py, &map)?;
             DirsMultiset::from_dirstate(&dirstate, skip_state)
         } else {
             let map: Result<Vec<HgPathBuf>, PyErr> = map
                 .iter(py)?
                 .map(|o| {
                     Ok(HgPathBuf::from_bytes(
                         o?.extract::<PyBytes>(py)?.data(py),
                     ))
                 })
                 .collect();
             DirsMultiset::from_manifest(&map?)
         };
 
         Self::create_instance(
             py,
             PySharedRefCell::new(inner),
         )
     }
 
     def addpath(&self, path: PyObject) -> PyResult<PyObject> {
         self.inner_shared(py).borrow_mut()?.add_path(
             HgPath::new(path.extract::<PyBytes>(py)?.data(py)),
         );
         Ok(py.None())
     }
 
     def delpath(&self, path: PyObject) -> PyResult<PyObject> {
         self.inner_shared(py).borrow_mut()?.delete_path(
             HgPath::new(path.extract::<PyBytes>(py)?.data(py)),
         )
             .and(Ok(py.None()))
             .or_else(|e| {
                 match e {
                     DirstateMapError::PathNotFound(_p) => {
                         Err(PyErr::new::<exc::ValueError, _>(
                             py,
                             "expected a value, found none".to_string(),
                         ))
                     }
                     DirstateMapError::EmptyPath => {
                         Ok(py.None())
                     }
                 }
             })
     }
     def __iter__(&self) -> PyResult<DirsMultisetKeysIterator> {
         let leaked_ref = self.inner_shared(py).leak_immutable()?;
         DirsMultisetKeysIterator::from_inner(
             py,
             unsafe { leaked_ref.map(py, |o| o.iter()) },
         )
     }
 
     def __contains__(&self, item: PyObject) -> PyResult<bool> {
-        Ok(self.inner(py).borrow().contains(HgPath::new(
+        Ok(self.inner_shared(py).borrow().contains(HgPath::new(
             item.extract::<PyBytes>(py)?.data(py).as_ref(),
         )))
     }
 });
 
 py_shared_ref!(Dirs, DirsMultiset, inner, inner_shared);
 
 impl Dirs {
     pub fn from_inner(py: Python, d: DirsMultiset) -> PyResult<Self> {
         Self::create_instance(py, PySharedRefCell::new(d))
     }
 
     fn translate_key(
         py: Python,
         res: &HgPathBuf,
     ) -> PyResult<Option<PyBytes>> {
         Ok(Some(PyBytes::new(py, res.as_ref())))
     }
 }
 
 py_shared_iterator!(
     DirsMultisetKeysIterator,
     PyLeakedRef<DirsMultisetIter<'static>>,
     Dirs::translate_key,
     Option<PyBytes>
 );

rust/hg-cpython/src/dirstate/dirstate_map.rs

@@ -1,504 +1,504 b''
 // dirstate_map.rs
 //
 // Copyright 2019 Raphaël Gomès <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 //! Bindings for the `hg::dirstate::dirstate_map` file provided by the
 //! `hg-core` package.
 
 use std::cell::{Ref, RefCell};
 use std::convert::TryInto;
 use std::time::Duration;
 
 use cpython::{
     exc, ObjectProtocol, PyBool, PyBytes, PyClone, PyDict, PyErr, PyObject,
     PyResult, PyTuple, Python, PythonObject, ToPyObject,
 };
 
 use crate::{
     dirstate::copymap::{CopyMap, CopyMapItemsIterator, CopyMapKeysIterator},
     dirstate::{dirs_multiset::Dirs, make_dirstate_tuple},
     ref_sharing::{PyLeakedRef, PySharedRefCell},
 };
 use hg::{
     utils::hg_path::{HgPath, HgPathBuf},
     DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap,
     DirstateParents, DirstateParseError, EntryState, StateMapIter,
     PARENT_SIZE,
 };
 
 // TODO
 //     This object needs to share references to multiple members of its Rust
 //     inner struct, namely `copy_map`, `dirs` and `all_dirs`.
 //     Right now `CopyMap` is done, but it needs to have an explicit reference
 //     to `RustDirstateMap` which itself needs to have an encapsulation for
 //     every method in `CopyMap` (copymapcopy, etc.).
 //     This is ugly and hard to maintain.
 //     The same logic applies to `dirs` and `all_dirs`, however the `Dirs`
 //     `py_class!` is already implemented and does not mention
 //     `RustDirstateMap`, rightfully so.
 //     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: PySharedRefCell<RustDirstateMap>;
 
     def __new__(_cls, _root: PyObject) -> PyResult<Self> {
         let inner = RustDirstateMap::default();
         Self::create_instance(
             py,
             PySharedRefCell::new(inner),
         )
     }
 
     def clear(&self) -> PyResult<PyObject> {
         self.inner_shared(py).borrow_mut()?.clear();
         Ok(py.None())
     }
 
     def get(
         &self,
         key: PyObject,
         default: Option<PyObject> = None
     ) -> PyResult<Option<PyObject>> {
         let key = key.extract::<PyBytes>(py)?;
-        match self.inner(py).borrow().get(HgPath::new(key.data(py))) {
+        match self.inner_shared(py).borrow().get(HgPath::new(key.data(py))) {
             Some(entry) => {
                 Ok(Some(make_dirstate_tuple(py, entry)?))
             },
             None => Ok(default)
         }
     }
 
     def addfile(
         &self,
         f: PyObject,
         oldstate: PyObject,
         state: PyObject,
         mode: PyObject,
         size: PyObject,
         mtime: PyObject
     ) -> PyResult<PyObject> {
         self.inner_shared(py).borrow_mut()?.add_file(
             HgPath::new(f.extract::<PyBytes>(py)?.data(py)),
             oldstate.extract::<PyBytes>(py)?.data(py)[0]
                 .try_into()
                 .map_err(|e: DirstateParseError| {
                     PyErr::new::<exc::ValueError, _>(py, e.to_string())
                 })?,
             DirstateEntry {
                 state: state.extract::<PyBytes>(py)?.data(py)[0]
                     .try_into()
                     .map_err(|e: DirstateParseError| {
                         PyErr::new::<exc::ValueError, _>(py, e.to_string())
                     })?,
                 mode: mode.extract(py)?,
                 size: size.extract(py)?,
                 mtime: mtime.extract(py)?,
             },
         );
         Ok(py.None())
     }
 
     def removefile(
         &self,
         f: PyObject,
         oldstate: PyObject,
         size: PyObject
     ) -> PyResult<PyObject> {
         self.inner_shared(py).borrow_mut()?
             .remove_file(
                 HgPath::new(f.extract::<PyBytes>(py)?.data(py)),
                 oldstate.extract::<PyBytes>(py)?.data(py)[0]
                     .try_into()
                     .map_err(|e: DirstateParseError| {
                         PyErr::new::<exc::ValueError, _>(py, e.to_string())
                     })?,
                 size.extract(py)?,
             )
             .or_else(|_| {
                 Err(PyErr::new::<exc::OSError, _>(
                     py,
                     "Dirstate error".to_string(),
                 ))
             })?;
         Ok(py.None())
     }
 
     def dropfile(
         &self,
         f: PyObject,
         oldstate: PyObject
     ) -> PyResult<PyBool> {
         self.inner_shared(py).borrow_mut()?
             .drop_file(
                 HgPath::new(f.extract::<PyBytes>(py)?.data(py)),
                 oldstate.extract::<PyBytes>(py)?.data(py)[0]
                     .try_into()
                     .map_err(|e: DirstateParseError| {
                         PyErr::new::<exc::ValueError, _>(py, e.to_string())
                     })?,
             )
             .and_then(|b| Ok(b.to_py_object(py)))
             .or_else(|_| {
                 Err(PyErr::new::<exc::OSError, _>(
                     py,
                     "Dirstate error".to_string(),
                 ))
             })
     }
 
     def clearambiguoustimes(
         &self,
         files: PyObject,
         now: PyObject
     ) -> PyResult<PyObject> {
         let files: PyResult<Vec<HgPathBuf>> = files
             .iter(py)?
             .map(|filename| {
                 Ok(HgPathBuf::from_bytes(
                     filename?.extract::<PyBytes>(py)?.data(py),
                 ))
             })
             .collect();
         self.inner_shared(py).borrow_mut()?
             .clear_ambiguous_times(files?, now.extract(py)?);
         Ok(py.None())
     }
 
     // TODO share the reference
     def nonnormalentries(&self) -> PyResult<PyObject> {
         let (non_normal, other_parent) =
-            self.inner(py).borrow().non_normal_other_parent_entries();
+            self.inner_shared(py).borrow().non_normal_other_parent_entries();
 
         let locals = PyDict::new(py);
         locals.set_item(
             py,
             "non_normal",
             non_normal
                 .iter()
                 .map(|v| PyBytes::new(py, v.as_ref()))
                 .collect::<Vec<PyBytes>>()
                 .to_py_object(py),
         )?;
         locals.set_item(
             py,
             "other_parent",
             other_parent
                 .iter()
                 .map(|v| PyBytes::new(py, v.as_ref()))
                 .collect::<Vec<PyBytes>>()
                 .to_py_object(py),
         )?;
 
         py.eval("set(non_normal), set(other_parent)", None, Some(&locals))
     }
 
     def hastrackeddir(&self, d: PyObject) -> PyResult<PyBool> {
         let d = d.extract::<PyBytes>(py)?;
         Ok(self.inner_shared(py).borrow_mut()?
             .has_tracked_dir(HgPath::new(d.data(py)))
             .to_py_object(py))
     }
 
     def hasdir(&self, d: PyObject) -> PyResult<PyBool> {
         let d = d.extract::<PyBytes>(py)?;
         Ok(self.inner_shared(py).borrow_mut()?
             .has_dir(HgPath::new(d.data(py)))
             .to_py_object(py))
     }
 
     def parents(&self, st: PyObject) -> PyResult<PyTuple> {
         self.inner_shared(py).borrow_mut()?
             .parents(st.extract::<PyBytes>(py)?.data(py))
             .and_then(|d| {
                 Ok((PyBytes::new(py, &d.p1), PyBytes::new(py, &d.p2))
                     .to_py_object(py))
             })
             .or_else(|_| {
                 Err(PyErr::new::<exc::OSError, _>(
                     py,
                     "Dirstate error".to_string(),
                 ))
             })
     }
 
     def setparents(&self, p1: PyObject, p2: PyObject) -> PyResult<PyObject> {
         let p1 = extract_node_id(py, &p1)?;
         let p2 = extract_node_id(py, &p2)?;
 
         self.inner_shared(py).borrow_mut()?
             .set_parents(&DirstateParents { p1, p2 });
         Ok(py.None())
     }
 
     def read(&self, st: PyObject) -> PyResult<Option<PyObject>> {
         match self.inner_shared(py).borrow_mut()?
             .read(st.extract::<PyBytes>(py)?.data(py))
         {
             Ok(Some(parents)) => Ok(Some(
                 (PyBytes::new(py, &parents.p1), PyBytes::new(py, &parents.p2))
                     .to_py_object(py)
                     .into_object(),
             )),
             Ok(None) => Ok(Some(py.None())),
             Err(_) => Err(PyErr::new::<exc::OSError, _>(
                 py,
                 "Dirstate error".to_string(),
             )),
         }
     }
     def write(
         &self,
         p1: PyObject,
         p2: PyObject,
         now: PyObject
     ) -> PyResult<PyBytes> {
         let now = Duration::new(now.extract(py)?, 0);
         let parents = DirstateParents {
             p1: extract_node_id(py, &p1)?,
             p2: extract_node_id(py, &p2)?,
         };
 
         match self.inner_shared(py).borrow_mut()?.pack(parents, now) {
             Ok(packed) => Ok(PyBytes::new(py, &packed)),
             Err(_) => Err(PyErr::new::<exc::OSError, _>(
                 py,
                 "Dirstate error".to_string(),
             )),
         }
     }
 
     def filefoldmapasdict(&self) -> PyResult<PyDict> {
         let dict = PyDict::new(py);
         for (key, value) in
             self.inner_shared(py).borrow_mut()?.build_file_fold_map().iter()
         {
             dict.set_item(py, key.as_ref().to_vec(), value.as_ref().to_vec())?;
         }
         Ok(dict)
     }
 
     def __len__(&self) -> PyResult<usize> {
-        Ok(self.inner(py).borrow().len())
+        Ok(self.inner_shared(py).borrow().len())
     }
 
     def __contains__(&self, key: PyObject) -> PyResult<bool> {
         let key = key.extract::<PyBytes>(py)?;
-        Ok(self.inner(py).borrow().contains_key(HgPath::new(key.data(py))))
+        Ok(self.inner_shared(py).borrow().contains_key(HgPath::new(key.data(py))))
     }
 
     def __getitem__(&self, key: PyObject) -> PyResult<PyObject> {
         let key = key.extract::<PyBytes>(py)?;
         let key = HgPath::new(key.data(py));
-        match self.inner(py).borrow().get(key) {
+        match self.inner_shared(py).borrow().get(key) {
             Some(entry) => {
                 Ok(make_dirstate_tuple(py, entry)?)
             },
             None => Err(PyErr::new::<exc::KeyError, _>(
                 py,
                 String::from_utf8_lossy(key.as_bytes()),
             )),
         }
     }
 
     def keys(&self) -> PyResult<DirstateMapKeysIterator> {
         let leaked_ref = self.inner_shared(py).leak_immutable()?;
         DirstateMapKeysIterator::from_inner(
             py,
             unsafe { leaked_ref.map(py, |o| o.iter()) },
         )
     }
 
     def items(&self) -> PyResult<DirstateMapItemsIterator> {
         let leaked_ref = self.inner_shared(py).leak_immutable()?;
         DirstateMapItemsIterator::from_inner(
             py,
             unsafe { leaked_ref.map(py, |o| o.iter()) },
         )
     }
 
     def __iter__(&self) -> PyResult<DirstateMapKeysIterator> {
         let leaked_ref = self.inner_shared(py).leak_immutable()?;
         DirstateMapKeysIterator::from_inner(
             py,
             unsafe { leaked_ref.map(py, |o| o.iter()) },
         )
     }
 
     def getdirs(&self) -> PyResult<Dirs> {
         // TODO don't copy, share the reference
         self.inner_shared(py).borrow_mut()?.set_dirs();
         Dirs::from_inner(
             py,
             DirsMultiset::from_dirstate(
-                &self.inner(py).borrow(),
+                &self.inner_shared(py).borrow(),
                 Some(EntryState::Removed),
             ),
         )
     }
     def getalldirs(&self) -> PyResult<Dirs> {
         // TODO don't copy, share the reference
         self.inner_shared(py).borrow_mut()?.set_all_dirs();
         Dirs::from_inner(
             py,
             DirsMultiset::from_dirstate(
-                &self.inner(py).borrow(),
+                &self.inner_shared(py).borrow(),
                 None,
             ),
         )
     }
 
     // TODO all copymap* methods, see docstring above
     def copymapcopy(&self) -> PyResult<PyDict> {
         let dict = PyDict::new(py);
-        for (key, value) in self.inner(py).borrow().copy_map.iter() {
+        for (key, value) in self.inner_shared(py).borrow().copy_map.iter() {
             dict.set_item(
                 py,
                 PyBytes::new(py, key.as_ref()),
                 PyBytes::new(py, value.as_ref()),
             )?;
         }
         Ok(dict)
     }
 
     def copymapgetitem(&self, key: PyObject) -> PyResult<PyBytes> {
         let key = key.extract::<PyBytes>(py)?;
-        match self.inner(py).borrow().copy_map.get(HgPath::new(key.data(py))) {
+        match self.inner_shared(py).borrow().copy_map.get(HgPath::new(key.data(py))) {
             Some(copy) => Ok(PyBytes::new(py, copy.as_ref())),
             None => Err(PyErr::new::<exc::KeyError, _>(
                 py,
                 String::from_utf8_lossy(key.data(py)),
             )),
         }
     }
     def copymap(&self) -> PyResult<CopyMap> {
         CopyMap::from_inner(py, self.clone_ref(py))
     }
 
     def copymaplen(&self) -> PyResult<usize> {
-        Ok(self.inner(py).borrow().copy_map.len())
+        Ok(self.inner_shared(py).borrow().copy_map.len())
     }
     def copymapcontains(&self, key: PyObject) -> PyResult<bool> {
         let key = key.extract::<PyBytes>(py)?;
         Ok(self
-            .inner(py)
+            .inner_shared(py)
             .borrow()
             .copy_map
             .contains_key(HgPath::new(key.data(py))))
     }
     def copymapget(
         &self,
         key: PyObject,
         default: Option<PyObject>
     ) -> PyResult<Option<PyObject>> {
         let key = key.extract::<PyBytes>(py)?;
         match self
-            .inner(py)
+            .inner_shared(py)
             .borrow()
             .copy_map
             .get(HgPath::new(key.data(py)))
         {
             Some(copy) => Ok(Some(
                 PyBytes::new(py, copy.as_ref()).into_object(),
             )),
             None => Ok(default),
         }
     }
     def copymapsetitem(
         &self,
         key: PyObject,
         value: PyObject
     ) -> PyResult<PyObject> {
         let key = key.extract::<PyBytes>(py)?;
         let value = value.extract::<PyBytes>(py)?;
         self.inner_shared(py).borrow_mut()?.copy_map.insert(
             HgPathBuf::from_bytes(key.data(py)),
             HgPathBuf::from_bytes(value.data(py)),
         );
         Ok(py.None())
     }
     def copymappop(
         &self,
         key: PyObject,
         default: Option<PyObject>
     ) -> PyResult<Option<PyObject>> {
         let key = key.extract::<PyBytes>(py)?;
         match self
             .inner_shared(py)
             .borrow_mut()?
             .copy_map
             .remove(HgPath::new(key.data(py)))
         {
             Some(_) => Ok(None),
             None => Ok(default),
         }
     }
 
     def copymapiter(&self) -> PyResult<CopyMapKeysIterator> {
         let leaked_ref = self.inner_shared(py).leak_immutable()?;
         CopyMapKeysIterator::from_inner(
             py,
             unsafe { leaked_ref.map(py, |o| o.copy_map.iter()) },
         )
     }
 
     def copymapitemsiter(&self) -> PyResult<CopyMapItemsIterator> {
         let leaked_ref = self.inner_shared(py).leak_immutable()?;
         CopyMapItemsIterator::from_inner(
             py,
             unsafe { leaked_ref.map(py, |o| o.copy_map.iter()) },
         )
     }
 
 });
 
 impl DirstateMap {
     pub fn get_inner<'a>(
         &'a self,
         py: Python<'a>,
     ) -> Ref<'a, RustDirstateMap> {
         self.inner_shared(py).borrow()
     }
     fn translate_key(
         py: Python,
         res: (&HgPathBuf, &DirstateEntry),
     ) -> PyResult<Option<PyBytes>> {
         Ok(Some(PyBytes::new(py, res.0.as_ref())))
     }
     fn translate_key_value(
         py: Python,
         res: (&HgPathBuf, &DirstateEntry),
     ) -> PyResult<Option<(PyBytes, PyObject)>> {
         let (f, entry) = res;
         Ok(Some((
             PyBytes::new(py, f.as_ref()),
             make_dirstate_tuple(py, entry)?,
         )))
     }
 }
 
 py_shared_ref!(DirstateMap, RustDirstateMap, inner, inner_shared);
 
 py_shared_iterator!(
     DirstateMapKeysIterator,
     PyLeakedRef<StateMapIter<'static>>,
     DirstateMap::translate_key,
     Option<PyBytes>
 );
 
 py_shared_iterator!(
     DirstateMapItemsIterator,
     PyLeakedRef<StateMapIter<'static>>,
     DirstateMap::translate_key_value,
     Option<(PyBytes, PyObject)>
 );
 
 fn extract_node_id(py: Python, obj: &PyObject) -> PyResult<[u8; PARENT_SIZE]> {
     let bytes = obj.extract::<PyBytes>(py)?;
     match bytes.data(py).try_into() {
         Ok(s) => Ok(s),
         Err(e) => Err(PyErr::new::<exc::ValueError, _>(py, e.to_string())),
     }
 }

rust/hg-cpython/src/ref_sharing.rs

@@ -1,500 +1,500 b''
 // ref_sharing.rs
 //
 // Copyright 2019 Raphaël Gomès <[e-mail]>
 //
 // Permission is hereby granted, free of charge, to any person obtaining a copy
 // of this software and associated documentation files (the "Software"), to
 // deal in the Software without restriction, including without limitation the
 // rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
 // sell copies of the Software, and to permit persons to whom the Software is
 // furnished to do so, subject to the following conditions:
 //
 // The above copyright notice and this permission notice shall be included in
 // all copies or substantial portions of the Software.
 //
 // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 // IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 // FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
 // AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 // LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
 // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
 // IN THE SOFTWARE.
 
 //! Macros for use in the `hg-cpython` bridge library.
 
 use crate::exceptions::AlreadyBorrowed;
 use cpython::{PyClone, PyObject, PyResult, Python};
 use std::cell::{Cell, Ref, RefCell, RefMut};
 
 /// Manages the shared state between Python and Rust
 #[derive(Debug, Default)]
 pub struct PySharedState {
     leak_count: Cell<usize>,
     mutably_borrowed: Cell<bool>,
 }
 
 // &PySharedState can be Send because any access to inner cells is
 // synchronized by the GIL.
 unsafe impl Sync for PySharedState {}
 
 impl PySharedState {
     pub fn borrow_mut<'a, T>(
         &'a self,
         py: Python<'a>,
         pyrefmut: RefMut<'a, T>,
     ) -> PyResult<PyRefMut<'a, T>> {
         if self.mutably_borrowed.get() {
             return Err(AlreadyBorrowed::new(
                 py,
                 "Cannot borrow mutably while there exists another \
                  mutable reference in a Python object",
             ));
         }
         match self.leak_count.get() {
             0 => {
                 self.mutably_borrowed.replace(true);
                 Ok(PyRefMut::new(py, pyrefmut, self))
             }
             // TODO
             // For now, this works differently than Python references
             // in the case of iterators.
             // Python does not complain when the data an iterator
             // points to is modified if the iterator is never used
             // afterwards.
             // Here, we are stricter than this by refusing to give a
             // mutable reference if it is already borrowed.
             // While the additional safety might be argued for, it
             // breaks valid programming patterns in Python and we need
             // to fix this issue down the line.
             _ => Err(AlreadyBorrowed::new(
                 py,
                 "Cannot borrow mutably while there are \
                  immutable references in Python objects",
             )),
         }
     }
 
     /// Return a reference to the wrapped data and its state with an
     /// artificial static lifetime.
     /// We need to be protected by the GIL for thread-safety.
     ///
     /// # Safety
     ///
     /// This is highly unsafe since the lifetime of the given data can be
     /// extended. Do not call this function directly.
     pub unsafe fn leak_immutable<T>(
         &self,
         py: Python,
         data: &PySharedRefCell<T>,
     ) -> PyResult<(&'static T, &'static PySharedState)> {
         if self.mutably_borrowed.get() {
             return Err(AlreadyBorrowed::new(
                 py,
                 "Cannot borrow immutably while there is a \
                  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;
         self.leak_count.replace(self.leak_count.get() + 1);
         Ok((&*ptr, &*state_ptr))
     }
 
     /// # Safety
     ///
     /// It's unsafe to update the reference count without knowing the
     /// reference is deleted. Do not call this function directly.
     pub unsafe fn decrease_leak_count(&self, _py: Python, mutable: bool) {
         if mutable {
             assert_eq!(self.leak_count.get(), 0);
             assert!(self.mutably_borrowed.get());
             self.mutably_borrowed.replace(false);
         } else {
             let count = self.leak_count.get();
             assert!(count > 0);
             self.leak_count.replace(count - 1);
         }
     }
 }
 
 /// `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>,
     py_shared_state: PySharedState,
 }
 
 impl<T> PySharedRefCell<T> {
     pub fn new(value: T) -> PySharedRefCell<T> {
         Self {
             inner: RefCell::new(value),
             py_shared_state: PySharedState::default(),
         }
     }
 
-    pub fn borrow(&self) -> Ref<T> {
+    pub fn borrow<'a>(&'a self, _py: Python<'a>) -> Ref<'a, 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()
     }
 
     // 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
     // refuses to borrow.
     pub fn borrow_mut<'a>(
         &'a self,
         py: Python<'a>,
     ) -> PyResult<PyRefMut<'a, T>> {
         self.py_shared_state.borrow_mut(py, self.inner.borrow_mut())
     }
 }
 
 /// Sharable data member of type `T` borrowed from the `PyObject`.
 pub struct PySharedRef<'a, T> {
     py: Python<'a>,
     owner: &'a PyObject,
     data: &'a PySharedRefCell<T>,
 }
 
 impl<'a, T> PySharedRef<'a, T> {
     /// # Safety
     ///
     /// The `data` must be owned by the `owner`. Otherwise, the leak count
     /// would get wrong.
     pub unsafe fn new(
         py: Python<'a>,
         owner: &'a PyObject,
         data: &'a PySharedRefCell<T>,
     ) -> Self {
         Self { py, owner, data }
     }
 
     pub fn borrow(&self) -> Ref<'a, T> {
-        self.data.borrow()
+        self.data.borrow(self.py)
     }
 
     pub fn borrow_mut(&self) -> PyResult<PyRefMut<'a, T>> {
         self.data.borrow_mut(self.py)
     }
 
     /// Returns a leaked reference.
     pub fn leak_immutable(&self) -> PyResult<PyLeakedRef<&'static T>> {
         let state = &self.data.py_shared_state;
         unsafe {
             let (static_ref, static_state_ref) =
                 state.leak_immutable(self.py, self.data)?;
             Ok(PyLeakedRef::new(
                 self.py,
                 self.owner,
                 static_ref,
                 static_state_ref,
             ))
         }
     }
 }
 
 /// Holds a mutable reference to data shared between Python and Rust.
 pub struct PyRefMut<'a, T> {
     inner: RefMut<'a, T>,
     py_shared_state: &'a PySharedState,
 }
 
 impl<'a, T> PyRefMut<'a, T> {
     // Must be constructed by PySharedState after checking its leak_count.
     // Otherwise, drop() would incorrectly update the state.
     fn new(
         _py: Python<'a>,
         inner: RefMut<'a, T>,
         py_shared_state: &'a PySharedState,
     ) -> Self {
         Self {
             inner,
             py_shared_state,
         }
     }
 }
 
 impl<'a, T> std::ops::Deref for PyRefMut<'a, T> {
     type Target = RefMut<'a, T>;
 
     fn deref(&self) -> &Self::Target {
         &self.inner
     }
 }
 impl<'a, T> std::ops::DerefMut for PyRefMut<'a, T> {
     fn deref_mut(&mut self) -> &mut Self::Target {
         &mut self.inner
     }
 }
 
 impl<'a, T> Drop for PyRefMut<'a, T> {
     fn drop(&mut self) {
         let gil = Python::acquire_gil();
         let py = gil.python();
         unsafe {
             self.py_shared_state.decrease_leak_count(py, true);
         }
     }
 }
 
 /// Allows a `py_class!` generated struct to share references to one of its
 /// data members with Python.
 ///
 /// # Warning
 ///
 /// TODO allow Python container types: for now, integration with the garbage
 ///     collector does not extend to Rust structs holding references to Python
 ///     objects. Should the need surface, `__traverse__` and `__clear__` will
 ///     need to be written as per the `rust-cpython` docs on GC integration.
 ///
 /// # Parameters
 ///
 /// * `$name` is the same identifier used in for `py_class!` macro call.
 /// * `$inner_struct` is the identifier of the underlying Rust struct
 /// * `$data_member` is the identifier of the data member of `$inner_struct`
 /// that will be shared.
 /// * `$shared_accessor` is the function name to be generated, which allows
 /// safe access to the data member.
 ///
 /// # Safety
 ///
 /// `$data_member` must persist while the `$name` object is alive. In other
 /// words, it must be an accessor to a data field of the Python object.
 ///
 /// # Example
 ///
 /// ```
 /// struct MyStruct {
 ///     inner: Vec<u32>;
 /// }
 ///
 /// py_class!(pub class MyType |py| {
 ///     data inner: PySharedRefCell<MyStruct>;
 /// });
 ///
 /// py_shared_ref!(MyType, MyStruct, inner, inner_shared);
 /// ```
 macro_rules! py_shared_ref {
     (
         $name: ident,
         $inner_struct: ident,
         $data_member: ident,
         $shared_accessor: ident
     ) => {
         impl $name {
             /// Returns a safe reference to the shared `$data_member`.
             ///
             /// This function guarantees that `PySharedRef` is created with
             /// the valid `self` and `self.$data_member(py)` pair.
             fn $shared_accessor<'a>(
                 &'a self,
                 py: Python<'a>,
             ) -> $crate::ref_sharing::PySharedRef<'a, $inner_struct> {
                 use cpython::PythonObject;
                 use $crate::ref_sharing::PySharedRef;
                 let owner = self.as_object();
                 let data = self.$data_member(py);
                 unsafe { PySharedRef::new(py, owner, data) }
             }
         }
     };
 }
 
 /// Manage immutable references to `PyObject` leaked into Python iterators.
 pub struct PyLeakedRef<T> {
     inner: PyObject,
     data: Option<T>,
     py_shared_state: &'static PySharedState,
 }
 
 // DO NOT implement Deref for PyLeakedRef<T>! Dereferencing PyLeakedRef
 // without taking Python GIL wouldn't be safe.
 
 impl<T> PyLeakedRef<T> {
     /// # Safety
     ///
     /// The `py_shared_state` must be owned by the `inner` Python object.
     // Marked as unsafe so client code wouldn't construct PyLeakedRef
     // struct by mistake. Its drop() is unsafe.
     pub unsafe fn new(
         py: Python,
         inner: &PyObject,
         data: T,
         py_shared_state: &'static PySharedState,
     ) -> Self {
         Self {
             inner: inner.clone_ref(py),
             data: Some(data),
             py_shared_state,
         }
     }
 
     /// Returns an immutable reference to the inner value.
     pub fn get_ref<'a>(&'a self, _py: Python<'a>) -> &'a T {
         self.data.as_ref().unwrap()
     }
 
     /// Returns a mutable reference to the inner value.
     ///
     /// Typically `T` is an iterator. If `T` is an immutable reference,
     /// `get_mut()` is useless since the inner value can't be mutated.
     pub fn get_mut<'a>(&'a mut self, _py: Python<'a>) -> &'a mut T {
         self.data.as_mut().unwrap()
     }
 
     /// Converts the inner value by the given function.
     ///
     /// Typically `T` is a static reference to a container, and `U` is an
     /// iterator of that container.
     ///
     /// # Safety
     ///
     /// The lifetime of the object passed in to the function `f` is cheated.
     /// It's typically a static reference, but is valid only while the
     /// corresponding `PyLeakedRef` is alive. Do not copy it out of the
     /// function call.
     pub unsafe fn map<U>(
         mut self,
         py: Python,
         f: impl FnOnce(T) -> U,
     ) -> PyLeakedRef<U> {
         // f() could make the self.data outlive. That's why map() is unsafe.
         // In order to make this function safe, maybe we'll need a way to
         // temporarily restrict the lifetime of self.data and translate the
         // returned object back to Something<'static>.
         let new_data = f(self.data.take().unwrap());
         PyLeakedRef {
             inner: self.inner.clone_ref(py),
             data: Some(new_data),
             py_shared_state: self.py_shared_state,
         }
     }
 }
 
 impl<T> Drop for PyLeakedRef<T> {
     fn drop(&mut self) {
         // py_shared_state should be alive since we do have
         // a Python reference to the owner object. Taking GIL makes
         // sure that the state is only accessed by this thread.
         let gil = Python::acquire_gil();
         let py = gil.python();
         if self.data.is_none() {
             return; // moved to another PyLeakedRef
         }
         unsafe {
             self.py_shared_state.decrease_leak_count(py, false);
         }
     }
 }
 
 /// Defines a `py_class!` that acts as a Python iterator over a Rust iterator.
 ///
 /// TODO: this is a bit awkward to use, and a better (more complicated)
 ///     procedural macro would simplify the interface a lot.
 ///
 /// # Parameters
 ///
 /// * `$name` is the identifier to give to the resulting Rust struct.
 /// * `$leaked` corresponds to `$leaked` in the matching `py_shared_ref!` call.
 /// * `$iterator_type` is the type of the Rust iterator.
 /// * `$success_func` is a function for processing the Rust `(key, value)`
 /// tuple on iteration success, turning it into something Python understands.
 /// * `$success_func` is the return type of `$success_func`
 ///
 /// # Example
 ///
 /// ```
 /// struct MyStruct {
 ///     inner: HashMap<Vec<u8>, Vec<u8>>;
 /// }
 ///
 /// py_class!(pub class MyType |py| {
 ///     data inner: PySharedRefCell<MyStruct>;
 ///
 ///     def __iter__(&self) -> PyResult<MyTypeItemsIterator> {
 ///         let leaked_ref = self.inner_shared(py).leak_immutable()?;
 ///         MyTypeItemsIterator::from_inner(
 ///             py,
 ///             unsafe { leaked_ref.map(py, |o| o.iter()) },
 ///         )
 ///     }
 /// });
 ///
 /// impl MyType {
 ///     fn translate_key_value(
 ///         py: Python,
 ///         res: (&Vec<u8>, &Vec<u8>),
 ///     ) -> PyResult<Option<(PyBytes, PyBytes)>> {
 ///         let (f, entry) = res;
 ///         Ok(Some((
 ///             PyBytes::new(py, f),
 ///             PyBytes::new(py, entry),
 ///         )))
 ///     }
 /// }
 ///
 /// py_shared_ref!(MyType, MyStruct, inner, MyTypeLeakedRef);
 ///
 /// py_shared_iterator!(
 ///     MyTypeItemsIterator,
 ///     PyLeakedRef<HashMap<'static, Vec<u8>, Vec<u8>>>,
 ///     MyType::translate_key_value,
 ///     Option<(PyBytes, PyBytes)>
 /// );
 /// ```
 macro_rules! py_shared_iterator {
     (
         $name: ident,
         $leaked: ty,
         $success_func: expr,
         $success_type: ty
     ) => {
         py_class!(pub class $name |py| {
             data inner: RefCell<Option<$leaked>>;
 
             def __next__(&self) -> PyResult<$success_type> {
                 let mut inner_opt = self.inner(py).borrow_mut();
                 if let Some(leaked) = inner_opt.as_mut() {
                     match leaked.get_mut(py).next() {
                         None => {
                             // replace Some(inner) by None, drop $leaked
                             inner_opt.take();
                             Ok(None)
                         }
                         Some(res) => {
                             $success_func(py, res)
                         }
                     }
                 } else {
                     Ok(None)
                 }
             }
 
             def __iter__(&self) -> PyResult<Self> {
                 Ok(self.clone_ref(py))
             }
         });
 
         impl $name {
             pub fn from_inner(
                 py: Python,
                 leaked: $leaked,
             ) -> PyResult<Self> {
                 Self::create_instance(
                     py,
                     RefCell::new(Some(leaked)),
                 )
             }
         }
     };
 }