diwic / reffers-rs

Rust wrappers around references, boxes and Arcs.
68 stars 3 forks source link

`ARef::map` is unsound #8

Closed steffahn closed 2 years ago

steffahn commented 2 years ago

Adaptation of Kimundi/owning-ref-rs#71, see that issue for an in-depth explanation, and ideas how to fix this problem

use reffers::ARef;

fn main() {
    let x = ARef::new(Box::new(()));
    let z: ARef<'static, str>;
    {
        let s = "Hello World!".to_string();
        let s_ref: &str = &s;
        let y: ARef<'static, &str> = x.map(|_| &s_ref);
        z = y.map(|s: &&str| *s);
        // s deallocated here
    }
    println!("{}", &*z); // printing garbage, accessing `s` after it’s freed
}
steffahn commented 2 years ago

Given that the suggested fix for OwningRef is to add a lifetime argument, but ARef already has one, I think the best fix would be just to use the same lifetime argument both for the owner lifetime and as a bound for the target type. So the fix would then be just to make it pub struct ARef<'a, U: 'a + ?Sized>, or equivalently change the _dummy to PhantomData<(Rc<()>, &'a U)>.

steffahn commented 2 years ago

Note FYI, that this new restriction on the lifetime parameter is arguably a breaking change, so 0.7.0 is probably going to be more appropriate than 0.6.2.