Is this use of transmute safe or are there weird edge cases?

code review
1

I'm currently implementing a kind of smart pointer that allows to track dropping of an entity.
It's intended to track changes made to entities loaded from db and only write back changes (including entities that were removed from a collection, without doing a bunch).

Currently I'm using a Option<Weak<dyn Tracker> to track deletion on drop.
With that design entities are able to outlive the db session.
I would like to use lifetimes to ensure that entities do not outlive the session without explicitly being detached.

The std library provides std::intrinsics::transmute, which can be used to extend or shorten the lifetime accordingly.
An implementation could look like this.
I've commented an alternative solution only using safe Rust.
The safe alternative has the problem that it needs to drop the original self, since Tracked<'t, E> and Tracked<'static, E> are two different types, but i feel like the "unsafe" variant should be safe?

This example on godbolt.org

use std::sync::Arc;

pub trait Tracker<E> {
    fn track(&self, entity: Arc<E>);
}

pub struct Tracked<'t, E> {
    inner: Arc<E>,
    tracker: Option<&'t dyn Tracker<E>>
}

impl<'t, E> Drop for Tracked<'t, E> {
    fn drop(&mut self) {
        if let Some(tracker) = self.tracker.as_ref() {
            tracker.track(self.inner.clone());
        }
    }
}

impl<'a, E> Tracked<'a, E> {
    pub fn register<'t>(mut self, tracker: &'t dyn Tracker<E>) -> Tracked<'t,E> {
        let mut s: Tracked<'t, E> = unsafe{ std::mem::transmute(self)};
        s.tracker = Some(tracker);
        s
        // self.tracker = None;
        // Tracked {
        //     inner: self.inner.clone(),
        //     tracker: Some(tracker)
        // }
    }
    pub fn deregister(mut self) -> Tracked<'static,E> {
        let mut s: Tracked<'static,E> = unsafe{ std::mem::transmute(self)};
        s.tracker = None;
        s
        // self.tracker = None;
        // Tracked {
        //     inner: self.inner.clone(),
        //     tracker: None        
        // }
    }
    
    pub fn new(inner: E) -> Self{
        Self {
            inner: Arc::new(inner),
            tracker: None
        }
    }

}

pub struct NoopTracker {}

impl<E> Tracker<E> for NoopTracker {
    
    fn track(&self, inner: Arc<E>) {}
}

pub fn main() { 

    let a = 0u8;
    let track = Tracked::new(a);
    let tracker = NoopTracker{};
    let track = track.register( &tracker);
    // drop(tracker); // borrow checker error here, as expected
    let track = track.deregister();
    drop(tracker);
    
 }
2

You're potentially creating references with lifetimes longer than their referent is valid. Set self.tracker to None before each transmute.

You can't call deregister as written unless E: 'static (as otherwise you'd have a non-well-formed type in the tracker field). It should return any lifetime 'long where E: 'long instead, so it can be called when E doesn't meet a 'static bound.

Once that's done, register can use unregister and be unsafe-free.

pub fn register<'t>(mut self, tracker: &'t dyn Tracker<E>) -> Tracked<'t,E> {
    let mut this = self.deregister();
    this.tracker = Some(tracker);
    this
}

pub fn deregister<'long>(mut self) -> Tracked<'long, E> where E: 'long {
    self.tracker = None;
    // SAFETY: Our only lifetime-carrying field is now `None`.
    unsafe { std::mem::transmute::<Self, Tracked<'long, E>>(self) }
}
1 Like
3

Thank you.

I've thought about ordering the clearing of the tracker before transmuting, but wasn't sure if it was necessary.
But it makes sense that it is.
Also the putting additional bound on E makes a lot of sense.
That just shows again that "unsafe" can be quite tricky, even for simple cases.

4

Since the reference didn't actually dangle, we're probably in safety-invariant and not validity-invariant territory, which would make it fine within unsafe... but since it's trivially avoidable, just avoid it :slight_smile:.

5

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.