diff -r 76a0bdb0e4ca -r 78b2894cd58c rust/pyo3-sharedref/src/lib.rs --- a/rust/pyo3-sharedref/src/lib.rs Mon Dec 16 12:13:46 2024 +0100 +++ b/rust/pyo3-sharedref/src/lib.rs Mon Dec 16 13:08:55 2024 +0100 @@ -469,15 +469,8 @@ /// may be exposed to your Rust code. You must be careful to not make a bare /// reference outlive the actual object lifetime. /// -/// ```ignore -/// let mut outer = empty; -/// let mut shared_iter = shared.map(py, |o| o.chars()); -/// { -/// let mut iter = unsafe { shared_iter.try_borrow_mut(py) }; -/// let inner = iter.next(); // Good, in borrow scope -/// outer = inner; // Bad, &'static T may outlive -/// } -/// ``` +/// See [`Self::try_borrow_mut()`] for an example of the kind of trouble that +/// can arise. pub struct SharedByPyObject { owner: PyObject, state: &'static PySharedState, @@ -498,11 +491,6 @@ // because the underlying value is supposed to be mutated in Python // world, and the Rust library designer can't prevent it. - // try_borrow() and try_borrow_mut() are unsafe because self.data may - // have a function returning the inner &'static reference. - // If T is &'static U, its lifetime can be easily coerced to &'a U, but - // how could we do that for Whatever<'static> in general? - /// Immutably borrows the wrapped value. /// /// Borrowing fails if the underlying reference has been invalidated. @@ -511,6 +499,49 @@ /// /// The lifetime of the innermost object is artificial. Do not obtain and /// copy it out of the borrow scope. + /// + /// The lifetime of the innermost object is artificial. Do not obtain and + /// copy it out of the borrow scope. More generally, the returned `&T` + /// may have a method returning an inner reference, which would typically + /// be `'static` and not safe without the `owner` Python object, so the + /// problem might be less obvious than in the example below. + /// + /// The following example does compile and illustrates the problem. + /// In this case, the data is a `Vec` and the leaked reference + /// `&'static str`, which points to some element of the vector. This + /// illustrates that the leaks are not necessarily to the whole of the + /// shared data. + /// + /// ```no_run + /// # use pyo3::prelude::*; + /// # use pyo3_sharedref::PyShareable; + /// #[pyclass] + /// struct Owner { + /// value: PyShareable> + /// } + /// + /// #[pymethods] + /// impl Owner { + /// #[new] + /// fn new(s: &str) -> Self { + /// let split: Vec<_> = s.split(' ').map(String::from).collect(); + /// Self { value: split.into() } + /// } + /// } + /// + /// const EMPTY: &'static str = ""; + /// + /// let mut outer = EMPTY; + /// Python::with_gil(|py| { + /// let owner = Bound::new(py, Owner::new("hello")).unwrap(); + /// let shareable = &owner.borrow().value; + /// let shared = unsafe { shareable.share(&owner) }; + /// { + /// let inner = unsafe { shared.try_borrow(py) }.unwrap(); + /// outer = &inner[0]; // Bad, &'static str does outlive the scope + /// } + /// }); + /// ``` pub unsafe fn try_borrow<'a>( &'a self, py: Python<'a>, @@ -526,13 +557,61 @@ /// /// Borrowing fails if the underlying reference has been invalidated. /// - /// Typically `T` is an iterator. If `T` is an immutable reference, - /// `get_mut()` is useless since the inner value can't be mutated. + /// Typically `T` would be an iterator obtained by the [`Self::map`] + /// method. /// /// # Safety /// /// The lifetime of the innermost object is artificial. Do not obtain and - /// copy it out of the borrow scope. + /// copy it out of the borrow scope. More generally, the returned `&T` + /// may have a method returning an inner reference, which would typically + /// be `'static` and not safe without the `owner` Python object, so the + /// problem might be less obvious than in the example below. + /// + /// The following example does compile and illustrates the problem. + /// It is very close to the example given in [`Self::try_borrow`] because + /// the problem does not arise from the mutability of the reference + /// returned by this function. + /// + /// In this case, the data is a `Vec` and the leaked reference + /// `&'static str`, which points to some element of the vector. This + /// illustrates that the leaks are not necessarily to the whole of the + /// shared data. + /// + /// ```no_run + /// # use pyo3::prelude::*; + /// # use pyo3_sharedref::PyShareable; + /// #[pyclass] + /// struct Owner { + /// value: PyShareable> + /// } + /// + /// #[pymethods] + /// impl Owner { + /// #[new] + /// fn new(s: &str) -> Self { + /// let split: Vec<_> = s.split(' ').map(String::from).collect(); + /// Self { value: split.into() } + /// } + /// } + /// + /// const EMPTY: &'static str = ""; + /// + /// let mut outer = EMPTY; + /// Python::with_gil(|py| { + /// let owner = Bound::new(py, Owner::new("hello")).unwrap(); + /// let shareable = &owner.borrow().value; + /// let shared = unsafe { shareable.share(&owner) }; + /// let mut shared_iter = unsafe { shared.map(py, |o| o.iter()) }; + /// { + /// let mut iter = unsafe { + /// shared_iter.try_borrow_mut(py) + /// }.unwrap(); + /// let inner = iter.next().unwrap(); // Good, in borrow scope + /// outer = inner; // Bad, &'static str does outlive the scope + /// } + /// }); + /// ``` pub unsafe fn try_borrow_mut<'a>( &'a mut self, py: Python<'a>,