On Tue, 22 Oct 2019 17:11:58 +0900, Yuya Nishihara wrote: > # HG changeset patch > # User Yuya Nishihara <y...@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), > } _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel