Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARef::map is unsound #8

Closed
steffahn opened this issue Jan 27, 2022 · 2 comments
Closed

ARef::map is unsound #8

steffahn opened this issue Jan 27, 2022 · 2 comments

Comments

@steffahn
Copy link

steffahn commented Jan 27, 2022

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
Copy link
Author

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)>.

@diwic diwic closed this as completed in 0d1ff1b Jan 28, 2022
@steffahn
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant