Lives - "lifetime-dynamic" smart pointers

code review
1

The goal is to create even "weaker" weak pointer, to allow it "forget" some covariant lifetimes and get them checked at runtime instead. In this way, we can store weak pointers of even disjoint lifetimes in the same container.

I personally don't feel like being a veteran in rust development, so feel free to tell me and discuss if you've found anything wrong.

Main idea

The standard library one, std::rc::Weak<T<'a>>, must have lifetime 'a attached to it since it's possible to be upgraded back to a std::rc::Rc<T<'a>>, which truly holds T<'a> and thus the lifetime must be tracked.

The main idea is: For type T<'a> which is covariant over 'a,.our version has reference-counting strong pointer lives::LifeRc<T<'a>> and weak pointer lives::LifeWeak<T<'static>> downgraded from the former one. Unlike the standard library one, we forbid upgrading LifeWeak back to the stronger LifeRc. Alternatively, from the LifeWeak side, all operation to the underlying &T<'a> must be done in the closure provided to the LifeWeak::with function.

To operate on the underlying &T<'a> from the LifeWeak side, at least one living strong pointer LifeRc<T<'a>> must be witnessed and kept alive (while operating on &T<'a>). Since LifeRc<T<'a>> cannot outlives 'a, we use it as a signal of aliveness of 'a during execution of LifeWeak::with. The checking here is indeed "dynamic".

Prior to granting the closure access to the &T<'a>, since the lifetime is masked to 'static in LifeWeak<T<'static>>, we must reconstruct it to some &'b T<'b> for 'b fully contained in 'a (that is, 'a: 'b). Since T<'a> is covariant over 'a and 'a: 'b, the operation on &'b T<'b> is sound.

What about invariant and contravariant lifetimes?

As is said, the smart pointers are for "forgetting" covariant lifetimes. However, it's not sound to construct &T<'a> to &'b T<'b> if T is invariant or contravariant over 'a. In that case, they must be explicitly kept, e.g., LifeRc<T<'covariant, 'invariant, 'contravariant>> -> LifeWeak<T<'static, 'invariant, 'contravariant>>.

2

this seems quite complex and i struggle to visualize an usecase, could you provide an example of a case where this used?

3

I personally use it when I want to keep track of some Box<dyn Trait> to dynamic dispatch, where the dynamic dispatcher is contextual and lives for static lifetime, while some implementer of the trait does not have static lifetime.

This might still seems vague, I think I will re-edit this answer when I have something more substantial.

4

If I understand correctly, you assume that if Rc<'a> still lives, then it must be valid to use 'a, so you can upgrade a Weak to it.

Unfortunately, this isn't true in all cases. It's possible to mem::forget(a_rc), and then outside of the scope of the lifetime resurrect a reference to it. This is unsound and easily reproducible in your example.

4 Likes
5

In case of me not getting to the point, does you mean it's safe in the case of Rc<T<'a>> verses Weak<T<'a>>, where the lifetime annotation prevent Weak<T<'a>> from outliving 'a?

6

Yes, lifetime on Weak<'a> prevents use out of scope of 'a without relying on what happens to Rc

1 Like
7

Thanks! I think this is worth mentioning in the documentation of this crate, as the limitation of our method.

8

this is more than a limitaton, this is an unsoundness which means that any user of your crate may do ub without realising. that would reqiring them to do a leak, of course, but a leak is not a great justification for UB.
one of LifeRc::new, LifeRc::Downgrade, and LifeWeak::with must be declared unsafe, with an unsafe documenation explaining that no LifeRc must ever be leaked for your code to be sound.

impl<T: Life> Drop for LifeRc<T> {
    fn drop(&mut self) {
        self.guard
            .try_borrow_mut()
            .expect("No LifeRc can be dropped during the execution of LifeWeak::with.");
      }
}

this is incorrect. it's very easy to do that.

fn panic() {
    let rc = LifeRc::new("a");
    let escaped = LifeRc::downgrade(&rc);
    escaped.with(move |r| {
        std::mem::drop(rc); // panic because of weird check in LifeRc::drop
        println!("{}", &*r);
    });
}

that being said, it's not a problem if a LifeRc is dropped during LifeWeak::with. that just means it was passed as an argument to the closure, which means T was valid when the closure was called, so it will be valid when the closure ends. simply removing the check is the way to go here.

also, you used derive(Clone). this was a mistake, as it ccauses you to require T : Clone for cloning, even though you do not need it. consider writing Clone by hand.

finally, it is usually is better to not put trait bounds on structs, unless they are necessery for layout or drop, which is not the case here.

pub struct LifeRc<T> {
    internal: Rc<T>,
    guard: Rc<RefCell<()>>,
}

impl<T> LifeRc<T> {
    pub fn new(t: T) -> Self {
        Self {
            internal: Rc::new(t),
            guard: Rc::new(RefCell::new(())),
        }
    }
}

impl<T> Clone for LifeRc<T> {

    fn clone(&self) -> Self {
       Self {
                internal : self.internal.clone(), 
                guard : self.guard.clone() 
               }
         }
}

impl<T> Deref for LifeRc<T> {
    type Target = T;

    fn deref(&self) -> &T {
        &*self.internal
    }
}

impl<T: Life> LifeRc<T> {
    /// Downgrade the reference-counting pointer
    /// to a weak pointer.
    /// SAFETY : the caller must guarantee that this `LifeRc` and all `LifeRc`s pointing to the same value will be dropped before `T` expires.
    /// this can simply be done by not leaking any of them, be it by `core::mem::leak`, `Box/Vec::leak`, an `Rc` reference cycle, or other such dangerous constuct. 
   ///it is safe to leak values in rust, so make sure to not use any API that doesn't guarantee it will not leak the values you give to it.
    pub unsafe  fn downgrade(rc: &LifeRc<T>) -> LifeWeak<T::Timed<'static>> {
        let weak = Rc::downgrade(&rc.internal);
        LifeWeak {
            internal: unsafe { std::mem::transmute(weak) },
            guard: rc.guard.clone(),
        }
    }
}
1 Like
9

Panicking here is intended rather than incorrect. I don't know why you would say it's not a problem if a LifeRc is dropped during LifeWeak::with. Reading the documentation of the crate, at the very beginning, it has said you must uphold the premise #2 that No LifeRc must ever be dropped during the execution of LifeWeak::with. It is a problem when you do that.

Imagine you are writing an async code, where you hold some resource &'a in a coroutine where T<'a> has access to, and send a LifeWeak to some caller outside the coroutine. Then in the with function, you first yield to the coroutine and let it run until T<'a> and &'a are out of scope, and access &'a with &T<'a> immediately in with, you will be going into UB. If you let LiveRc::drop here, it prevent the code from further chaos by UB by screaming right at the time your code is incorrect at runtime. This is similar to when you call borrow or borrow_mut of a RefCell, it checks broken usage at the runtime.

10

you cannot yield from anything inside with, as it takes a closure.

if a LifeRc is dropped during the execution of F, then necessarily F must capture LifeRc<T> in its type (the true T), therefore T must outlive F, and it is guaranteeed that the value is safe to access for the whole execution of with.
so actually, even if you supported an async_with, where you could yield this wouldn't be useful for anything.

i hope this was enough to convince you, but if you do think i am incorrect, feel free to make a counter example, like the following for your current real UB problem.

use lives::LifeRc;
fn use_after_free_ub() {
    let escaped;
    {
        let a = format!("{}", 123456);
        let rc = LifeRc::new(a.as_str());
        escaped = LifeRc::downgrade(&rc);
        core::mem::forget(rc)
    }
    escaped.with(|s| (**s).to_string()); //use after free
}
```
11 
[dependencies]
lives = { version = "0.1.2" }
futures =  { version = "0.3.31" }

use std::{cell::RefCell, rc::Rc};
use futures::task::LocalSpawnExt;
use lives::{Life, LifeRc, LifeWeak};
use futures::channel::oneshot::{self, Receiver};
use futures::executor::LocalPool;

#[derive(Life)]
struct Str<'a>(&'a str);

struct Holder {
    weaks: Vec<LifeWeak<Str<'static>>>,
}

async fn async_work(
    holder: Rc<RefCell<Holder>>,
    recv: Receiver<()>,
) {
    let resource = format!("Something");
    let s = Str(&resource);
    let rc = LifeRc::new(s);
    let weak = LifeRc::downgrade(&rc);
    holder.borrow_mut().weaks.push(weak);
    let _ = recv.await;
}

fn main() {
    let holder = Rc::new(RefCell::new(Holder{
        weaks: Vec::new(),
    }));

    let mut pool = LocalPool::new();

    let (send, recv) = oneshot::channel();
    pool.spawner().spawn_local(async_work(holder.clone(), recv)).unwrap();
    pool.run_until_stalled();

    let weak = holder.borrow_mut().weaks.pop().unwrap();
    weak.with(|v| {
        let _ = send.send(());
        pool.run_until_stalled(); // You are yielding to the coroutine here.
        println!("{}", v.0); // <-- UB here if LifeRc::drop didn't panic.
    });
}
12

ok so i two things :

  • amazing job for catching that. i have to admit, it is so foreign from how lifetimes normally work i did not see it coming.
  • i have bad news, your fix is not enough :
struct PrintOnDrop<'a>(&'a str);

impl<'a> Drop for PrintOnDrop<'a> {
    fn drop(&mut self) {
        println!("{}", &*self.0)
    }
}

fn main() {
    let holder = Rc::new(RefCell::new(Holder{
        weaks: Vec::new(),
    }));

    let mut pool = LocalPool::new();

    let (send, recv) = oneshot::channel();
    pool.spawner().spawn_local(something_wierd(holder.clone(), recv)).unwrap();
    pool.run_until_stalled();

    let weak = holder.borrow_mut().weaks.pop().unwrap();
    weak.with(|v| {
        let p = PrintOnDrop(v.0);
        let _ = send.send(());
        pool.run_until_stalled(); // <- panic here. panics leads to UB
        println!("{}", v.0); // no UB here because panic
    });
}
13

pretty sure the only valid fix is aborting.

if you don't care about UB during unwinding, remember catch_unwind is a thing, and it is used.


    weak.with(|v| {
        let _ = send.send(());
        let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| pool.run_until_stalled()));
        println!("{}", &*v.0); // <-- UB actually happes here
    });
1 Like
14

Agreed.

15

After reflecting on what is happening here, and the possibility of leaking and forgetting LifeRc (as leaking is safe in rust, which I've underlooked, clearly), I agree it's necessary to make downgrade unsafe to serve as a last remainder of opening the Pandora's box.

And thank you for reviewing the code, it takes time and effort to do that.

1 Like