__init and __pin_init don't know the lifetime of the created object

[bonzini]

Imagine a function like this that initializes an object through an external C API:

impl Timer {
    pub fn init<'timer, 'opaque: 'timer, T>(self: Pin<&'timer mut Self>, f: fn(&T), opaque: &'opaque T)  {
        // call to C API
    }
}

When converting this to use PinInit, it is not possible to confirm that the opaque value outlives the timer:

impl Timer {
    pub fn init<T>(f: fn(&T), opaque: *mut T) -> impl PinInit<Self> {
        pin_init_from_closure(move |slot: *mut Timer|
            // call to C API
        }
    }
}

If PinInit received for example an &'a MaybeUninit<T>, it would be possible to do add the constraint to the signature:

impl Timer {
    pub fn init<'timer, 'opaque: 'timer, T: 'opaque>(f: fn(&T), opaque: &'opaque mut T)
                -> impl PinInit<'timer, Self> {
        unsafe {
            pin_init_from_closure(move |slot: &'timer mut MaybeUninit<Timer>| {
                // call to C API
            })
        }
    }
}

MaybeUninit is not suitable (for example it does not support unsized types); but it should be possible to wrap a *mut T and constrain it to a generic lifetime:

pub struct ToInit<'a, T>(*mut T, PhantomData<&'a mut MaybeUninit<T>>);

impl Timer {
    pub fn init<'timer, 'opaque: 'timer, T: 'opaque>(f: fn(&T), opaque: ToInit<'opaque, T>)
                -> impl PinInit<'timer, Self> {
        unsafe {
            pin_init_from_closure(move |slot: ToInit<'timer, Timer>| {
                // call to C API
            })
        }
    }
}

This arises in QEMU, which implements callbacks using (roughly) function pointers instead of traits like Linux.

[bonzini]

(I will send this to the Rust-for-Linux mailing list as well once I have something more like a prototype).

[y86-dev]

I don't think it's a good idea to add a lifetime to PinInit, since that will have to be carried everywhere...

I also think it's possible to do this today without any changes:

impl Timer {
    pub fn init<'timer, 'opaque: 'timer, T: 'opaque>(
        f: fn(&T),
        opaque: &'opaque mut T,
    ) -> impl PinInit<Self> + 'timer {
        unsafe {
            pin_init_from_closure(|slot| {
                // call to C API
            })
        }
    }
}

IIUC, you only want to capture the lifetime of 'timer and thus ensure that this code is not allowed:

fn foo() -> Pin<Box<Timer>> {
    let t;
    {
        let mut opaque = ();
        t = Timer::init(|_| (), &mut opaque);
    }
    Box::pin_init(t).unwrap()
}

this is exactly what the example I gave above prevents.

However, if you just try to write

impl Timer {
    pub fn init<'timer, 'opaque: 'timer, T: 'opaque>(
        f: fn(&T),
        opaque: &'opaque mut T,
    ) -> impl PinInit<Self> {
        unsafe {
            pin_init_from_closure(|slot| {
                // call to C API
            })
        }
    }
}

Then foo also doesn't compile with the same error, so maybe there is more that you need?

[bonzini]

I also think it's possible to do this today without any changes:

impl Timer {
    pub fn init<'timer, 'opaque: 'timer, T: 'opaque>(
        f: fn(&T),
        opaque: &'opaque mut T,
    ) -> impl PinInit<Self> + 'timer {

The problem is that I don't yet have a fully initialized T, and therefore I have neither a &mut T nor a lifetime.

The idea is that I am initializing the outer object and storing a timer into it; the outer object acts as the opaque. Timer::init()'s knows that it's okay to store a pointer to an uninitialized opaque, because it's not going to be read until all the initialization has finished.

What I want to do is allow this:

    pin_init!(&this in Struct { timer <- Timer::init(Struct::timer_cb, this })

but forbid this:

    pin_init!(&this in Struct { timer <- Timer::init(Struct::timer_cb, some_other_pointer_to_struct })

[y86-dev]

Ah I see what you want to do... hmm can you share a bit more why you need to access the outer struct in the Timer? Do you want to store a pointer to the parent inside of some callback instead of the address of the timer? In the kernel, we use container_of! for that and that works reasonably well.

[bonzini]

It's just the way the C API works—instead of using container_of in the callback, QEMU has timer initialization take a void* in addition to the timer. It does "waste" a word per timer, but it's how it was designed 20 years ago. :) It does work without needing the complex set of traits that exist in Linux for work items and hrtimers, so it's got some advantages(*).

It's not the end of the world if initialization is partly unsafe but more in general, having lifetimes available might make pinned_init more applicable to other usecases than wrapping C APIs. For example, initializing cyclical data structures safely: imagine a ToInit::write that—unlike pointer write and like MaybeUninit—is safe because the type system and borrow checker enforce that it is called exactly once in the init closure.

I see your point about tracking the lifetime everywhere; even if it can be written as '_, ToInit<'_, T> is a bit longer than *mut T. Perhaps we could use some kind of IntoXxx trait, so that the init closure could take either a raw pointer and a ToInit<...>. If it's okay for you to leave the issue open, I will find some time to prototype it.

---

(*) The Linux container_of mechanism also does not extend as well to arrays of timers; say I have a t: [Mutex<HPETTimer>; HPET_MAX_TIMERS] array in my struct, each of which has aTimer inside, I can pass the &Mutex<HPETTimer> as the opaque object and just initialize the timer with timer::init(timer_callback, mutex). That makes it easy to lock the one object that fired. With Linux's container_of strategy it's of course possible—for example you can place the timer callback on a newtype for Mutex<HPETTimer>—but I find it more complex.

[y86-dev]

It's just the way the C API works—instead of using container_of in the callback, QEMU has timer initialization take a void* in addition to the timer. It does "waste" a word per timer, but it's how it was designed 20 years ago. :) It does work without needing the complex set of traits that exist in Linux for work items and hrtimers, so it's got some advantages(*).

I see.

It's not the end of the world if initialization is partly unsafe but more in general, having lifetimes available might make pinned_init more applicable to other usecases than wrapping C APIs. For example, initializing cyclical data structures safely: imagine a ToInit::write that—unlike pointer write and like MaybeUninit—is safe because the type system and borrow checker enforce that it is called exactly once in the init closure.

yeah that sounds like a good idea (and like another application for field projection rust-lang/rfcs#3735)

I see your point about tracking the lifetime everywhere; even if it can be written as '_, ToInit<'_, T> is a bit longer than *mut T. Perhaps we could use some kind of IntoXxx trait, so that the init closure could take either a raw pointer and a ToInit<...>.

The ToInit<'_, T> type is not the part that I worry about. Instead I don't want other users of PinInit to have to write impl PinInit<'_, T, E>. Since the pin_init_from_closure is a low level API anyways, I also don't mind changing the argument of its closure parameter.

I'm not 100% opposed to adding the lifetime to PinInit<T>, but I'd have to check if it's possible to elide it completely (i.e. continue writing impl PinInit<T, E>, even though there is a lifetime). In that case, I think it's a possibility.

Another solution would be to introduce a new type that functions exactly like PinInit, except that it has the lifetime.

If it's okay for you to leave the issue open, I will find some time to prototype it.

Sure!

(*) The Linux container_of mechanism also does not extend as well to arrays of timers; say I have a t: [Mutex<HPETTimer>; HPET_MAX_TIMERS] array in my struct, each of which has aTimer inside, I can pass the &Mutex<HPETTimer> as the opaque object and just initialize the timer with timer::init(timer_callback, mutex). That makes it easy to lock the one object that fired. With Linux's container_of strategy it's of course possible—for example you can place the timer callback on a newtype for Mutex<HPETTimer>—but I find it more complex.

Yeah both approaches have their (dis)advantages.

[y86-dev]

I see your point about tracking the lifetime everywhere; even if it can be written as '_, ToInit<'_, T> is a bit longer than *mut T. Perhaps we could use some kind of IntoXxx trait, so that the init closure could take either a raw pointer and a ToInit<...>.

The ToInit<'_, T> type is not the part that I worry about. Instead I don't want other users of PinInit to have to write impl PinInit<'_, T, E>. Since the pin_init_from_closure is a low level API anyways, I also don't mind changing the argument of its closure parameter.

I'm not 100% opposed to adding the lifetime to PinInit<T>, but I'd have to check if it's possible to elide it completely (i.e. continue writing impl PinInit<T, E>, even though there is a lifetime). In that case, I think it's a possibility.

Another solution would be to introduce a new type that functions exactly like PinInit, except that it has the lifetime.

Oh maybe I have understood you incorrectly here, I don't think that we need to add a lifetime to PinInit<T, E> at all, we only need to add it to the pin_init_from_closure function.

So essentially, we only need the ToInit<'a, T> type and change the signature of the closure. Though I think we should keep __init and __pinned_init the same... maybe? I don't know, that's probably something that one finds out if they actually try to implement it. If you want to try it, give it a go!

[bonzini]

I don't think that we need to add a lifetime to PinInit<T, E> at all, we only need to add it to the pin_init_from_closure function.

Hah you're right - though before edition 2024 you'll need a use<'timer, T, E, F> or something like that. Thanks for the hint, I'll keep it in mind.

[y86-dev]

Can we close this issue? Or do you need something added to pin-init?