Carrying owner and struct borrows it together

Repo: Fancyflame/Any-ref-rs: Creating a bundle that can carry an owner and a struct borrows it (github.com)

This crate is similar to owning_ref, but it could carry custom struct rather than references only. This crate used unsafe codes, thus I hope someone could review it and give me some advice to improve. Thanks!

Note that owning_ref itself has lots of unsound API. (And appears to be rather unmaintained at the moment.) Feel free to look through the open issues.

I see. But I think owning_ref's goal is sound, my lib is not based on owning_ref and the principle of realization is much differ from owning_ref.

Yes, the goal of owning_ref should be sound; I just wanted to make you aware that there are problems and those may or may not apply to your crate too.

Also let me say that getting safe abstractions for self-referencing stuff is insanely hard; there’s just so many ways to mess up and make the API unsound, since Rust has no understanding of self-referencing types itself, so its usual safety tools won’t be of any help.

These are just some general remarks; I might get around to actually reviewing your crate a bit later. Asking for review is 100% the correct approach here ^^.

1 Like

I'm so appreciate that you could review. And yes, making self-referencing is hard in rust, so my API unavoidably a bit not comfortable, I'll try best to improve. But the most significant is safety and performance.

use any_ref::{make_any_ref, AnyRef, Reference};

fn main() {
    let x: AnyRef<Reference<str>, Box<str>> = AnyRef::new("Hello".into(), |x| x);
    let y: AnyRef<RefRef<str>, Box<str>> = x.map(|x| x);
    println!("{}", y.get());
}

make_any_ref! {
    type RefRef<T: 'static + ?Sized> = for<'a> &'a &'a T;
}
cargo run --release
[1]    367654 segmentation fault (core dumped)
2 Likes

I've been running your demo, but this problem won't appear on my machine, so I can only guess the reason.
I think the problem is in the map function, which has a type declaration F: for<'a> FnOnce(&'a <T as ReturnType<'a>>::Target) -> <T2 as ReturnType<'a>>::Target. The problem is, when calling map, the given argument &'a <T as ReturnType<'a>>::Target actually doesn't satisfy the lifetime annotation at &'a, but the mistake was hidden by unsafe.


I think the argument should not be borrowed. I have tried to resolve it but encountered a compile error. I modified &'a <T as ReturnType<'a>>::Target to <T as ReturnType<'a>, so the function now looks like this:

    pub fn map<T2, F>(self, func: F) -> AnyRef<T2, O>
    where
        T2: LifetimeDowncast + ?Sized,
        F: for<'a> FnOnce(<T as ReturnType<'a>>::Target) -> <T2 as ReturnType<'a>>::Target,
    {
        AnyRef {
            holder: func(self.holder),
            owner: self.owner,
        }
    }

But it couldn't compile. I've tried many ways but neither succeeded.

error[E0582]: binding for associated type `Output` references lifetime `'a`, which does not appear in the trait input types
  --> src\lib.rs:98:61
   |
98 |         F: for<'a> FnOnce(<T as ReturnType<'a>>::Target) -> <T2 as ReturnType<'a>>::Target,
   |                                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The declaration of ReturnType is here:

pub trait ReturnType<'a> {
    type Target: 'a;
}

Could you please give me some hints to resolve it? I think it should be possible because ReturnType<'a>::Target indeed has lifetime annotation.

To reproduce the segfault with @steffahn's program, one must run cargo run --release with a nightly version no older than nightly-2022-03-09. Bisection reveals that rustc started allowing the program with the PR https://github.com/rust-lang/rust/pull/90887, which should be merged into 1.61.0.

I haven't looked at your crate or considered any soundness issues.

Regarding the immediate question though, it looks like #49601, in which case you might be able to work around it with a helper trait.

Yes I know this bug in the stable version, yet I provided the function new_any_ref as an alternative proposal. I mean here is I reproduced @steffahn's demo but no segfault occurred, but this is still an underlying problem, which needs to be settled.

Using helper trait indeed could let the 'map` function be compiled, but cannot be used, even in the nightly toolchain.

error: implementation of `FnOnce` is not general enough
  --> src/lib.rs:23:7
   |
23 |     s.map::<T,_>(|x|x);
   |       ^^^ implementation of `FnOnce` is not general enough
   |
   = note: closure with signature `fn(<T as ReturnType<'2>>::Target) -> <T as ReturnType<'_>>::Target` must implement `FnOnce<(<T as ReturnType<'1>>::Target,)>`, for any lifetime `'1`...
   = note: ...but it actually implements `FnOnce<(<T as ReturnType<'2>>::Target,)>`, for some specific lifetime `'2`

Here's a case where it can be used. I'm unsure if the closure case is a higher-ranked inference problem (notorious on closures), or some deeper limitation.

Indeed, but I want T not to be limited to static types. I wonder if there is a workaround even using unsafe, as long as I can keep it safe.

Guess this problem can't be resolved immediately, I changed

for<'a> FnOnce(<T as ReturnType<'a>>::Target) -> <T2 as ReturnType<'a>>::Target

to

for<'a> FnOnce(<T as ReturnType<'a>>::Target, &'a <O as Deref>::Target) -> <T2 as ReturnType<'a>>::Target

as a compromised workaround.

The program segfaults on my system with cargo +nightly run --release, even when I replace AnyRef::new with new_any_ref. Could you please post the output of rustc +nightly -V -v?

I'm sorry but I have pushed new codes to github, and it's inconvenient for me to reproduce. Now you'll be prevent by the compiler from doing that thing.

I remembered the behavior on my machine is just printing "hello" with debug mode. But triggering segfault is expected in this demo.

Additionally, new_any_ref and AnyRef::new is exactly the same, there is no need to test twice.

I'm appreciate if you can help finding other bugs. :smiley:

If you have trouble with reproducing the segfault, then you could try running it with miri?

1 Like

The Clone impl for AnyRef is unsound:

use any_ref::{make_any_ref, new_any_ref, AnyRef};
use std::rc::Rc;

fn main() {
    let source: Rc<Box<str>> = Rc::new("Hello".into());
    let x: AnyRef<EvilReference<Box<str>>, Rc<Box<str>>> =
        new_any_ref(source, |x| EvilRef::new(x)).clone();
    let y: &'static Box<str> = x.get().leaked.unwrap();
    drop(x);
    let _garbage = Box::new([1usize; 4]);
    println!("{y}");
}

struct EvilRef<'a, T: 'static + ?Sized> {
    inner: &'a T,
    leaked: Option<&'static T>,
}

impl<T: ?Sized> EvilRef<'_, T> {
    fn new(inner: &T) -> EvilRef<T> {
        EvilRef {
            inner,
            leaked: None,
        }
    }
}

impl<T: ?Sized> Clone for EvilRef<'static, T> {
    fn clone(&self) -> Self {
        EvilRef {
            inner: self.inner,
            leaked: Some(self.inner),
        }
    }
}

make_any_ref! {
    type EvilReference<T: 'static + ?Sized> = for<'a> EvilRef<'a, T>;
}
$ cargo +nightly run
   Compiling wat v0.1.0 (/home/lm978/wat)
    Finished dev [unoptimized + debuginfo] target(s) in 0.17s
     Running `target/debug/wat`
Segmentation fault (core dumped)
1 Like

Well, interesting and good demo, this bug seems to be originate from my inadvertent mistake, already fixed.
Thanks for your effort :smiley:.

Fixed segfault occurred when cloning AnyRef · Fancyflame/Any-ref-rs@22b79ed (github.com)

Just noticed, you got the order of fields wrong, the owner is dropped first!

use any_ref::{make_any_ref, AnyRef};

struct AccessOnDrop<'a>(&'a str);

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

make_any_ref! {
    type StrRefWithDropAccess = for<'a> AccessOnDrop<'a>;
}

fn main() {
    AnyRef::<StrRefWithDropAccess, Box<str>>::new("Hello World!".into(), |x| AccessOnDrop(x));
}

Thanks, I've never noticed such details. :pray:t2: