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