Is this self-referential struct safe?

1

I was looking at this question and was curious what it would look like solving it using owning_ref. I ended up using OwningHandle since the reference Proxy is its own type. I replaced the dbus types with fake types in the example below just to avoid the dbus dependency since I'm on Windows.

My question has to do with the deref to internal types with 'static lifetimes for a self referential type. That item is wrapped in a reference so its not like you can really get to the inner lifetime (I think). I would think its valid as long as the object its in is valid which is what owning_ref should get you. I think its allowed by the compiler due to the way the library uses pointers internally. But I can't say I'm 100% its fine.

use std::ops::Deref;
use std::path::Path;
use owning_ref::OwningHandle;

struct SelfDeref<T>(T);

impl<T> std::ops::Deref for SelfDeref<T> {
    type Target = T;
    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

struct Connection(usize);

impl Connection {
    fn new_session() -> Result<Self, ()> {
        let i = rand::random();
        Ok(Connection(i))
    }

    fn with_proxy<'a, 'b>(&'a self, path: &'b Path) -> Proxy<'b, &'a Self> {
        Proxy { path, conn: self }
    }
}
struct Proxy<'a, C> {
    path: &'a Path,
    conn: C,
}

struct WrappedProxy(OwningHandle<Box<Connection>, SelfDeref<Proxy<'static, &'static Connection>>>);

// impl Deref for WrappedProxy {
//     type Target = OwningHandle<Box<Connection>, SelfDeref<Proxy<'static, &'static Connection>>>;
//     fn deref(&self) -> &Self::Target {
//         &self.0
//     }
// }

impl Deref for WrappedProxy {
    type Target = Proxy<'static, &'static Connection>;
    fn deref(&self) -> &Self::Target {
        &self.0.deref()
    }
}

impl WrappedProxy {
    fn new() -> Self {
        let conn = Connection::new_session().unwrap();
        let proxy = OwningHandle::new_with_fn(Box::new(conn), |conn| {
            let conn = unsafe { conn.as_ref().unwrap() };
            SelfDeref(conn.with_proxy(Path::new("/my/path")))
        });
        WrappedProxy(proxy)
    }
}
2

No, by taking a copy of the connection field, you can obtain a &'static Connection that points to non-static memory.

The thing that implements Deref may point into the box, but the thing it derefs to may not.

3

As soon as you find yourself reaching for unsafe or lifetime tick marks it time to step back and have a serious look at what you are trying to do. Neither should be necessary for any normal application level code. Or at least I have not found a need for them in my year of Rust.

The Rust compiler is trying to stop you sticking your hand in the fire. You are trying hard to stick you hand in the fire anyway.

4

You’re talking about Proxy::conn? If the privacy of that field was suitably restricted such that all access was through functions that re-attach an appropriate lifetime, would that be sound? Something like:

impl Proxy {
    fn get_conn<‘a>(&’a self)->&’a Connection { &self.conn }
}
5

Yes, it is possible to make it sound by suitably restricting the lifetimes of things you can access. In fact, that's how owning_ref is able to be sound in the first place.

6

Are you referring to something in my code since I don't think I take a copy of it? Or are you saying that if someone took connection and cloned it (assuming that was possible) it would be a problem?

What I don't get though, is that if the compiler is okay with this even if I attached lifetimes to Deref, it seems like they won't necessarily be connected to anything. For the example below, I just don't know what those lifetimes mean for a self referential struct.

struct WrappedProxy<'a,'c>(OwningHandle<Box<Connection>, SelfDeref<Proxy<'a, &'c Connection>>>);

impl <'a,C> std::ops::Deref for WrappedProxy<'a,C> {
    type Target = Proxy<'a, C>;
    fn deref(&self) -> &Self::Target {
        &self.0.deref()
    }
}

I guess I'm struggling to see how to use deref in this case to anything other than path or connection directly which means you can't pick up any methods defined on Proxy using deref.

7

No, what I am saying is that using the code you posted can accidentally result in a use-after-free, but there is not actually a use-after-free in the code itself. Normally you would want ordinary use of the WrappedProxy object to be protected from memory errors.

Additionally, you are not making use of any of the features provided by owning_ref, and could just as easily replace it with a struct with two fields and a similar unsafe block.

They are indeed not attached to anything. It is allowed because 'static is allowed, and since all other lifetimes are shorter than 'static, any lifetime is allowed.

8

Whenever you want unsafe code to be audited, you should be very explicit about what is pub and what isn't. A pattern I like to do when sharing in forums / Discord and the like is to do:

mod lib { // privacy (and thus safety)  boundary
    pub ...

    // other non-`pub` stuff
}

This way, if somebody outside lib is able to cause UB without using the word unsafe, then your abstraction is definitely unsound. Otherwise, if enough code reviews fail to see the unsoundness, and/or if you can prove that your abstraction is correct (e.g., by documenting the different invariants), then it should be fine.

Back to the OP, we can assume that you wish to make WrappedProxy be pub. Since trait impls are as public as the trait they implement, Proxy<'static, &'static Connection> would be pub too,

If the conn field of Proxy was private, and instead the only way to access it was through some
fn conn<'proxy> (self: &'proxy Self) -> &'proxy Connection getter, then, w.r.t. to that Connection access, and based on OwningRef being sound (which it currently technically isn't according to Stacked Borrows, since it won't use an aliasable Box newtype), your construct ought to be sound.

9

Thank you both @Yandros and @alice per usual. What you both said makes sense. I think I got initially thrown off with it compiling at all which I wasn’t expecting.