D6631: rust-cpython: add macro for sharing references
Yuya Nishihara
yuya at tcha.org
Sat Aug 17 04:38:46 EDT 2019
> +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 with an artificial static
> + /// lifetime.
> + /// We need to be protected by the GIL for thread-safety.
> + pub fn leak_immutable<T>(
> + &self,
> + py: Python,
> + data: &RefCell<T>,
> + ) -> PyResult<&'static T> {
> + if self.mutably_borrowed.get() {
> + return Err(AlreadyBorrowed::new(
> + py,
> + "Cannot borrow immutably while there is a \
> + mutable reference in Python objects",
> + ));
> + }
> + let ptr = data.as_ptr();
> + self.leak_count.replace(self.leak_count.get() + 1);
> + unsafe { Ok(&*ptr) }
> + }
This leak_immutable() looks potentially unsafe since it can extend the
lifetime of an arbitrary object, data.
> +macro_rules! py_shared_ref {
> + (
> + $name: ident,
> + $inner_struct: ident,
> + $data_member: ident,
> + $leaked: ident,
> + ) => {
> + impl $name {
> + fn borrow_mut<'a>(
> + &'a self,
> + py: Python<'a>,
> + ) -> PyResult<crate::ref_sharing::PyRefMut<'a, $inner_struct>>
> + {
> + self.py_shared_state(py)
> + .borrow_mut(py, self.$data_member(py).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))
> + }
> + }
Is it okay to directly borrow/borrow_mut() the inner object?
For example, we can still do `self.inner(py).borrow_mut()` (in place of
`self.borrow_mut(py)`) while the inner object is leaked, which I think is
unsafe.
If I understand the py-shared logic correctly, PySharedState needs to own
the inner object to guarantee that any public operations are safe.
More information about the Mercurial-devel
mailing list