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

[BennoLossin]

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

[BennoLossin]

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.

[BennoLossin]

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.

[BennoLossin]

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.

[BennoLossin]

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

[bonzini]

Basically I would need something like pin_init's PinUninit<'_, Self> type.

The relationship between the crates.io and Linux pin_init is not clear to me - if I should just use pin_init and pin_project this issue can certainly be closed.

Otherwise, it's probably too late to change all the Linux code, so you can close it either, but it would be helpful to know which case is true. :)

[BennoLossin]

Basically I would need something like pin_init's PinUninit<'_, Self> type.

Interesting 🤔 Feel free to open a new issue, as that seems different to the stuff that you mentioned in this issue.

The relationship between the crates.io and Linux pin_init is not clear to me - if I should just use pin_init and pin_project this issue can certainly be closed.

The original idea was to merge the crates and only have pin-init. That is taking some time, since we want to use syn before we do the merger. We hopefully get it soon in the kernel. After that pinned-init will no longer be updated and you should switch to pin-init.

@nbdd0121 and I have to think about how to exactly do the merging of the crates, as there are certain features (such as pin-project compatibility) that we have to think about. Would be great to have some information from users what they actually need :)

[bonzini]

Basically I would need something like pin_init's PinUninit<'_, Self> type.

that seems different to the stuff that you mentioned in this issue.

It's related, in that PinUninit<'a, T> is like Pin<&'a mut MaybeUninit<T>>. But you're right, it is not enough because in pin-init it's __init that takes the lifetime, not the trait:

pub trait Init<T, E>: Sized {
    fn __init<'a>(self, this: PinUninit<'a, T>) -> InitResult<'a, T, E>;

and there is no way to constrain 'a. :( This is the same reason why this is just NonNull<Self> and doesn't have a lifetime.

That said, I probably won't have time to work on this so it can be closed.

[BennoLossin]

I'm still not sure I understand what you're trying to do. Feel free to point me to the code that you're trying to write and I can take a look at some point.