[PATCH 7 of 7] rust-cpython: mark all PyLeaked methods as unsafe

[Yuya Nishihara]

On Tue, 22 Oct 2019 17:11:58 +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya at tcha.org>
> # Date 1571727874 -32400
> #      Tue Oct 22 16:04:34 2019 +0900
> # Node ID 779af7c0979780d03451dd7cfe303e8dc64c6576
> # Parent  29399437267567957a97dc9d924704ca748e3cec
> rust-cpython: mark all PyLeaked methods as unsafe

CC: Raphaël, Georges

This might be interesting. The other patches in this series are refactoring
stuff.

> Unfortunately, these methods can be abused to obtain the inner 'static
> reference. The simplest (pseudo-code) example is:
> 
>   let leaked: PyLeaked<&'static _> = shared.leak_immutable();
>   let static_ref: &'static _ = &*leaked.try_borrow(py)?;
>       // PyLeakedRef::deref() tries to bound the lifetime to itself, but
>       // the underlying data is a &'static reference, so the returned
>       // reference can be &'static.
> 
> This problem can be easily fixed by coercing the lifetime, but there are
> many other ways to achieve that, and there wouldn't be a generic solution:
> 
>   let leaked: PyLeaked<&'static [_]> = shared.leak_immutable();
>   let leaked_iter: PyLeaked<slice::Iter<'static, _>>
>       = unsafe { leaked.map(|v| v.iter()) };
>   let static_slice: &'static [_] = leaked_iter.try_borrow(py)?.as_slice();
> 
> So basically I failed to design the safe borrowing interface. Maybe we'll
> instead have to add much more restricted interface on top of the unsafe
> PyLeaked methods? For instance, Iterator::next() could be implemented if
> its Item type is not &'a (where 'a may be cheated.)
> 
> Anyway, this seems not an easy issue, so it's probably better to leave the
> current interface as unsafe, and get broader comments while upstreaming this
> feature.
> 
> 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
> @@ -294,7 +294,13 @@ impl<T> PyLeaked<T> {
>      /// Immutably borrows the wrapped value.
>      ///
>      /// Borrowing fails if the underlying reference has been invalidated.
> -    pub fn try_borrow<'a>(
> +    ///
> +    /// # Safety
> +    ///
> +    /// The lifetime of the innermost object is cheated. Do not obtain and
> +    /// copy it out of the borrow scope. See the example of `try_borrow_mut()`
> +    /// for details.
> +    pub unsafe fn try_borrow<'a>(
>          &'a self,
>          py: Python<'a>,
>      ) -> PyResult<PyLeakedRef<'a, T>> {
> @@ -311,7 +317,24 @@ impl<T> PyLeaked<T> {
>      ///
>      /// 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 try_borrow_mut<'a>(
> +    ///
> +    /// # Safety
> +    ///
> +    /// The lifetime of the innermost object is cheated. Do not obtain and
> +    /// copy it out of the borrow scope. For example, the following code
> +    /// is unsafe:
> +    ///
> +    /// ```compile_fail
> +    /// let slice;
> +    /// {
> +    ///     let iter = leaked.try_borrow_mut(py);
> +    ///     // slice can outlive since the iterator is of Iter<'static, T>
> +    ///     // type, but it shouldn't.
> +    ///     slice = iter.as_slice();
> +    /// }
> +    /// println!("{:?}", slice);
> +    /// ```
> +    pub unsafe fn try_borrow_mut<'a>(
>          &'a mut self,
>          py: Python<'a>,
>      ) -> PyResult<PyLeakedRefMut<'a, T>> {
> @@ -423,6 +446,11 @@ impl<T> DerefMut for PyLeakedRefMut<'_, 
>  /// tuple on iteration success, turning it into something Python understands.
>  /// * `$success_func` is the return type of `$success_func`
>  ///
> +/// # Safety
> +///
> +/// `$success_func` may take a reference, but it's lifetime may be cheated.
> +/// Do not copy it out of the function call.
> +///
>  /// # Example
>  ///
>  /// ```
> @@ -476,9 +504,10 @@ macro_rules! py_shared_iterator {
>  
>              def __next__(&self) -> PyResult<$success_type> {
>                  let mut leaked = self.inner(py).borrow_mut();
> -                let mut iter = leaked.try_borrow_mut(py)?;
> +                let mut iter = unsafe { leaked.try_borrow_mut(py)? };
>                  match iter.next() {
>                      None => Ok(None),
> +                    // res may be a reference of cheated 'static lifetime
>                      Some(res) => $success_func(py, res),
>                  }