(Solved) Future in union, is that sound?

[coral fractal]

Full context can be found in the github repository:
https://github.com/ljsnogard/snapshot-channel-rs/blob/dev/0.0.1/src/snapshot_.rs#L194

I store a future and its output into a union.

union FutOrOut<F>
where
    F: Future,
{
    future_: ManuallyDrop<F>,
    output_: ManuallyDrop<F::Output>,
}

impl<F> FutOrOut<F>
where
    F: Future,
{
    pub unsafe fn pinned_future(self: Pin<&mut Self>) -> Pin<&mut F> {
        let this = unsafe { self.get_unchecked_mut() };
        unsafe { Pin::new_unchecked(this.future_.deref_mut()) }
    }
    pub unsafe fn output_ref(&self) -> &F::Output {
        self.output_.deref()
    }
    pub unsafe fn output_mut(&mut self) -> &mut F::Output {
        self.output_.deref_mut()
    }
}

The state of FutOrOut is indicated by other bit flags.

Then, I have:

let mut fut_or_out: Pin<&mut FutOrOut<F>> = (*fut_guard).as_mut();
let fut: Pin<&mut F> = unsafe { fut_or_out.as_mut().pinned_future() };

// future polling here
let poll_res = fut.poll(cx);
let Poll::Ready(x) = poll_res
else {
    // irrelevant
};

// future got overwrite here, is that sound?
unsafe {
    let p = fut_or_out.get_unchecked_mut();
    *(p.output_mut()) = x;
}

No crash encountered, but is that sound?

[keen eagle]

I don't see any obvious problems here. Except ofc I'd first ask if this is really necessary? can you not just use an enum? i would hope that the future has niches available anyway.

Besides that, you're not dropping the future, which is allowed, but probably not what you want.

oh uh, I guess you're technically violating the Pin invariant at the end there

maybe? not sure actually

[coral fractal]

Thanks for the reply.It used to be an enum, but, the problem is, I have to atomically change the variant. So a bit flag as the variant indicator is a better choice, I think.

[coral fractal]

That's what I want to figure out. I think the code has fully in charge of the lifetime of a future, is it neccessary to drop the future? I am not sure either.

[keen eagle]

Does that nake sense? you can atomically change the variant, but wouldn't you also have to atomically change the union then? otherwise they are out of sync

[keen eagle]

dropping the future is definitely something you should do if you can

otherwise you're potentially leaking memory everywhere

that's not unsound, but probably not what you want

[coral fractal]

1. Yes, you are right. Dropping the future is absolutely needed, that's what I missed
2. If FutOrOut is an enum, there is a subtle time gap, between, when the future is polled, turned ready, and when the output written to FutOrOut, the code I wrote for that confusing myself :-(. Maybe I should have a try again.

[keen eagle]

ok, so regarding the code as written: why are you pinning fut_or_out? I think what you should be doing is only pin the &mut fut for the duration of polling

[keen eagle]

It feels like you're overcomplicating something here. if you need help with the bigger problem, I'm happy to help :)

[twin blaze]

yeah getting xy vibes

if you are concerned about time gaps when atomically changing things, it sounds like your orderings aren't done properly (unless you are sensitive to at most microseconds delay)

[coral fractal]

@keen eagle @twin blaze Thank you all for the advices and replies.
So I changed FutOrRes from union to enum, added a simple unit test to see if the future can properly drop.
https://github.com/ljsnogard/snapshot-channel-rs/blob/dev/0.0.1/src/glimpse_.rs#L352