# HG changeset patch # User Yuya Nishihara # Date 1569053750 -32400 # Node ID f8c114f20d2d4850a4ee8d6c01af555251cb449e # Parent ffc1fbd7d1f5bb719f7fc1c35b4affdc7b8ae907 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. 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 @@ -100,7 +100,7 @@ } def __contains__(&self, item: PyObject) -> PyResult { - Ok(self.inner(py).borrow().contains(HgPath::new( + Ok(self.inner_shared(py).borrow().contains(HgPath::new( item.extract::(py)?.data(py).as_ref(), ))) } 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 @@ -63,7 +63,7 @@ default: Option = None ) -> PyResult> { let key = key.extract::(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)?)) }, @@ -170,7 +170,7 @@ // TODO share the reference def nonnormalentries(&self) -> PyResult { 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( @@ -281,18 +281,18 @@ } def __len__(&self) -> PyResult { - Ok(self.inner(py).borrow().len()) + Ok(self.inner_shared(py).borrow().len()) } def __contains__(&self, key: PyObject) -> PyResult { let key = key.extract::(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 { let key = key.extract::(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)?) }, @@ -333,7 +333,7 @@ Dirs::from_inner( py, DirsMultiset::from_dirstate( - &self.inner(py).borrow(), + &self.inner_shared(py).borrow(), Some(EntryState::Removed), ), ) @@ -344,7 +344,7 @@ Dirs::from_inner( py, DirsMultiset::from_dirstate( - &self.inner(py).borrow(), + &self.inner_shared(py).borrow(), None, ), ) @@ -353,7 +353,7 @@ // TODO all copymap* methods, see docstring above def copymapcopy(&self) -> PyResult { 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()), @@ -365,7 +365,7 @@ def copymapgetitem(&self, key: PyObject) -> PyResult { let key = key.extract::(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::( py, @@ -378,12 +378,12 @@ } def copymaplen(&self) -> PyResult { - Ok(self.inner(py).borrow().copy_map.len()) + Ok(self.inner_shared(py).borrow().copy_map.len()) } def copymapcontains(&self, key: PyObject) -> PyResult { let key = key.extract::(py)?; Ok(self - .inner(py) + .inner_shared(py) .borrow() .copy_map .contains_key(HgPath::new(key.data(py)))) @@ -395,7 +395,7 @@ ) -> PyResult> { let key = key.extract::(py)?; match self - .inner(py) + .inner_shared(py) .borrow() .copy_map .get(HgPath::new(key.data(py))) 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 @@ -136,7 +136,7 @@ } } - pub fn borrow(&self) -> Ref { + 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. @@ -180,7 +180,7 @@ } pub fn borrow(&self) -> Ref<'a, T> { - self.data.borrow() + self.data.borrow(self.py) } pub fn borrow_mut(&self) -> PyResult> {