`TimeCapsule`&`FrozenFuture` is an unsound API, reliant on users to `.await` the `.freeze(&mut …)` future properly in-place

[steffahn]

So, well, TimeCapsule<T> can accept a reference of just any lifetime for freeze. It produces a FrozenFuture<'a, 'b, T> that does have some lifetime corresponding to that reference, but all that ensures is that the reference stays alive until that future is polled, nothing else. If you do end up just .awaiting that future in-place (in particular, converting the Poll::pending of FrozenFuture into an immediately propagated Poll::pending of the surrounding future), then all may be well, but who forces you to do this?

That’s an easy exploit: Just poll the future, then invalidate the reference, and only then “propagate” the Poll::pending. Below is a straightforward implementation of this using helper macros from the futures crate family:

use std::marker::PhantomData;
use nolife::{BoxScope, Family, TimeCapsule};
use futures_util::{poll, pending};

struct SingleFamily<T: 'static>(PhantomData<T>);
impl<'a, T: 'static> Family<'a> for SingleFamily<T> {
    type Family = T;
}

fn main() {
    {
        let mut scope = BoxScope::new(
            |mut time_capsule: TimeCapsule<SingleFamily<String>>| async move {
                let mut x = String::from("Hello World!");
                let fut = time_capsule.freeze(&mut x);
                let _ = poll!(fut); // puts reference into state
                drop(x); // then invalidates reference
                pending!(); // then yields
                panic!();
            },
        );
        scope.enter(|s| {
            println!("{s}");
        })
    }
}

outputs garbage data, e.g.:

b����

miri output

error: Undefined Behavior: out-of-bounds pointer use: alloc877 has been freed, so this pointer is dangling
   --> /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/raw.rs:109:9
    |
109 |         &*ptr::slice_from_raw_parts(data, len)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: alloc877 has been freed, so this pointer is dangling
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
help: alloc877 was allocated here:
   --> src/main.rs:14:29
    |
14  |                 let mut x = String::from("Hello World!");
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: alloc877 was deallocated here:
   --> src/main.rs:17:17
    |
17  |                 drop(x); // then invalidates reference
    |                 ^^^^^^^
    = note: BACKTRACE (of the first span):
    = note: inside `std::slice::from_raw_parts::<'_, u8>` at /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/raw.rs:109:9: 109:47
    = note: inside `<std::vec::Vec<u8> as std::ops::Deref>::deref` at /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:2709:18: 2709:64
    = note: inside `<std::string::String as std::ops::Deref>::deref` at /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/string.rs:2539:43: 2539:52
    = note: inside `<std::string::String as std::fmt::Display>::fmt` at /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/string.rs:2354:28: 2354:34
    = note: inside `<&mut std::string::String as std::fmt::Display>::fmt` at /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:2298:62: 2298:82
    = note: inside `core::fmt::rt::Argument::<'_>::fmt` at /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/rt.rs:142:9: 142:40
    = note: inside `std::fmt::write` at /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:1120:17: 1120:40
    = note: inside `<std::io::StdoutLock<'_> as std::io::Write>::write_fmt` at /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/mod.rs:1846:15: 1846:43
    = note: inside `<&std::io::Stdout as std::io::Write>::write_fmt` at /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/stdio.rs:736:9: 736:36
    = note: inside `<std::io::Stdout as std::io::Write>::write_fmt` at /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/stdio.rs:710:9: 710:33
    = note: inside `std::io::stdio::print_to::<std::io::Stdout>` at /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/stdio.rs:1029:21: 1029:47
    = note: inside `std::io::_print` at /home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/stdio.rs:1106:5: 1106:37
note: inside closure
   --> src/main.rs:23:13
    |
23  |             println!("{s}");
    |             ^^^^^^^^^^^^^^^
    = note: inside `nolife::scope::Scope::<SingleFamily<std::string::String>, {async block@src/main.rs:13:67: 20:14}>::enter::<'_, (), {closure@src/main.rs:22:21: 22:24}>` at /home/frank/.cargo/registry/src/index.crates.io-6f17d22bba15001f/nolife-0.3.3/src/scope.rs:166:22: 166:30
    = note: inside `nolife::BoxScope::<SingleFamily<std::string::String>, {async block@src/main.rs:13:67: 20:14}>::enter::<'_, (), {closure@src/main.rs:22:21: 22:24}>` at /home/frank/.cargo/registry/src/index.crates.io-6f17d22bba15001f/nolife-0.3.3/src/box_scope.rs:117:18: 117:41
note: inside `main`
   --> src/main.rs:22:9
    |
22  | /         scope.enter(|s| {
23  | |             println!("{s}");
24  | |         })
    | |__________^
    = note: this error originates in the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

---

Unfortunately, I cannot immediately think of straightforward fixes for soundness here.¹ This might need more thought though. Let’s hope there is some alternative approach out there, and that his doesn’t mean this whole crate’s idea is fundamentally broken.

Footnotes

1. At least without an API that involves some mandatory usage of a macro that calls unsafe, possibly-hidden, API. ↩

[dureuill]

Hello @steffahn,

Thank you for taking the time to examine nolife, I'm super happy to see you here 😃

I was looking for this kind of reviews earlier, and I thought that announcing the crate through a blogpost would have more chances to give it some visibility, basically as an application of Cunningham's Law.

Now, I'm not super happy with the outcome 😅 but such are the risks of trying a new approach.

Back to this issue, my understanding of the root cause is that you can decorrelate polling the FrozenFuture (which setups the executor's State) from actually yielding back to the executor.

Possible fixes would entail:

1. Force the returned borrow to live for the duration of the outer future. That's probably limiting the expressivity to a point where nolife wouldn't be useful.
2. Force yielding back to the executor upon polling the FrozenFuture somehow.

(2) is not even that simple to implement, because there can be multiple levels of future, so a simple freeze!() macro that would compile down to time_capsule.freeze(&mut x).await() would be insufficient:

use std::marker::PhantomData;
use nolife::{BoxScope, Family, TimeCapsule};
use futures_util::{poll, pending};

struct SingleFamily<T: 'static>(PhantomData<T>);
impl<'a, T: 'static> Family<'a> for SingleFamily<T> {
    type Family = T;
}

async fn inner(time_capsule: TimeCapsule<SingleFamily<String>>, data: &mut String) {
     nolife::freeze!(time_capsule, data);
} 

fn main() {
    {
        let mut scope = BoxScope::new(
            |mut time_capsule: TimeCapsule<SingleFamily<String>>| async move {
                let mut x = String::from("Hello World!");
                let fut = inner(time_capsule, &mut x);
                let _ = poll!(fut); // puts reference into state
                drop(x); // then invalidates reference
                pending!(); // then yields
                panic!();
            },
        );
        scope.enter(|s| {
            println!("{s}");
        })
    }
}

Or is it some other macro you had in mind?

Of course we could add a drop impl to FrozenFuture that resets the state, but then I guess the next step would be to forget about the future.

[steffahn]

(2) is not even that simple to implement, because there can be multiple levels of future, so a simple freeze!() macro that would compile down to time_capsule.freeze(&mut x).await() would be insufficient:

Yes indeed, that's insufficient, of course. The macro would need to do all of

• make sure the time_capsule.freeze…APIs are only available at the top-level of the block and only in a way immediately followed byawait`
• wrap that block in an async (or at least parse an async { } a user wrote) so that it knows what counts as “top-level” to begin with
• pass that async on to the actual BoxScope::new call, so that the future itself couldn’t otherwise be polled, then dropped, before yield

Even then, I’m not 100% sure yet what the best design could be to allow time_capsule.freeze…().await calls at the top-level. Searching for literal occurrences of “time_capsule” seems tedious (and not even necessarily sound in the presence of other macros that could piece together such an identifier; though I’d need to double check what guarantees / protection hygiene could offer). Having a helper macro available (or defined) inside of the async block might be able to once again couple the call with the .await, but does little to enforce this top-level-ness, either.

Perhaps, ultimately, the outer macro would need to generate the .freeze().await/.freeze_forever().await calls itself. A minimally flexible (but maximally simple) API would just name have you name an identifier foo and have the macro insert a time_capsule. Ultimately I think there are many ways to structure such a macro, with different levels of difficulty in terms of parsing the contents of a block, and different flexibility of API that can be offered.

[steffahn]

Oh, I’m having some interesting macro ideas now…

[dureuill]

Just a thought, could we make the situation better by marking the FrozenFuture as !Unpin?

A !Unpin future cannot be poll!ed. Now, that's not sufficient, because one could just Box::pin the FrozenFuture (or even stack pin it since the value is owned), but that's a step in a desirable direction: make it unsound to pin the FrozenFuture by anyone else than the top-level executor.

[steffahn]

Oh, I’m having some interesting macro ideas now…

use nolife::Family;
use std::marker::PhantomData;

struct SingleFamily<T: 'static>(PhantomData<T>);
impl<'a, T: 'static> Family<'a> for SingleFamily<T> {
    type Family = T;
}

macro_rules! scope {
    {$($t:tt)*} => {{
        ::nolife::BoxScope::new(|mut time_capsule: ::nolife::TimeCapsule<SingleFamily<String>>| async move {
            'check_freeze_scope: {
                #[allow(unused_macros)]
                macro_rules! freeze {
                    ($e:expr) => {{
                        #[allow(unreachable_code)]
                        if false {
                            break 'check_freeze_scope (loop {});
                        }
                        ::nolife::TimeCapsule::freeze(&mut time_capsule, $e).await
                    }};
                }
                #[allow(unused_macros)]
                macro_rules! freeze_forever {
                    ($e:expr) => {{
                        #[allow(unreachable_code)]
                        if false {
                            break 'check_freeze_scope (loop {});
                        }
                        ::nolife::TimeCapsule::freeze_forever(&mut time_capsule, $e).await
                    }};
                }
                {
                    $($t)*
                }
            }
        })
    }}
}

fn main() {
    let mut sc = scope! {
        let mut x = String::from("Hello World!");
        let _fut = async {
            // freeze!(&mut x); // usage in nested future errors
            #[allow(unused_labels)]
            'check_freeze_scope: {
                // freeze!(&mut x); // even here, thanks to hygiene
            }
        };

        freeze_forever!(&mut x)
    };
    sc.enter(|s| {
        println!("{s}");
    })
}

this kind of approach might force all the constraints. (Some of the relevant API, either the BoxScope::new or the freeze methods would just need to become unsafe, and the macro becomes a safe wrapper.)

(I haven’t tested to what extend replacing the ::nolife prefixes with $crate:: works for macros defined inside of macros, or whether $crate then refers to the using crate, not to nolife itself).

[steffahn]

In case you don’t want to include the BoxScope::new call with the macro, e.g. to also allow for alternative choices like ::pin, then you could have scope! return some ScopeProducer<P> wrapper that’s otherwise unsafe to construct, and make new and pin expect such a wrapped producer: ScopeProducer<P> instead of producer: P as argument.

[dureuill]

Hmmm, that looks like a very smart solution, I also like the resulting interface a lot! I really, truly, appreciate you taking of reviewing this crate and providing helpful solutions this way ❤️

Am I correct that this obviously limits the ability to factor the scope in multiple functions?

Consider a more complex example including error handling like this gist. Do we need to write macros for subscopes now :D ?

[dureuill]

(I haven’t tested to what extend replacing the ::nolife prefixes with $crate:: works for macros defined inside of macros, or whether $crate then refers to the using crate, not to nolife itself).

I just tested this: $crate resolves to the crate of the outer macro definition, meaning you can use $crate everywhere in your macro definitions there, even the inner macros, and it will do the right thing (i.e. resolve to nolife).

[steffahn]

Am I correct that this obviously limits the ability to factor the scope in multiple functions?

Yes.

Consider a more complex example including error handling like this gist. Do we need to write macros for subscopes now :D ?

I wonder if there’s a way to create some PropagatePendingToScope<F> wrapper for futures. And a macro to create an async block in which other PropagatePendingToScope<F> blocks could be executed. And the scope takes a TimeCapsule<…> -> PropagatePendingToScope<F> argument, whereas freeze isn’t an async fn but an fn(…) -> PropagatePendingToScope<impl Future<…>>.

I think that doesn’t prevent “time capsule juggling” where you could smuggle the time capsule of an outer scope into an inner scope, in order to write to it (to the outer scope) without propagating the Pending after all. This in turn could probably be solved by introducing a unique invariant lifetime, in order to tag both the TimeCapsule and the PropagatePendingToScope so that they must always correspond.

I don’t have the time today to turn this into a concrete implementation, but I believe it could possibly be sound (and not much worse in UX than the macros above), whilst allowing factoring out helper functions as desired.

[dureuill]

Alright I think I'm starting to see a bit clearer how that would work at multiple levels.

1. At the top level there is a BoxScope::new(ScopeProducer<P>).
2. The scope! macro accepts a tt and turns it into a ScopeProducer.
3. In the scope! macro, you get 3 superpowers:
   1. freeze!(e) that checks it is not in an inner future using your scope trick, then TimeCapsule::freeze(&mut input_time_capsule, $e)
   2. freeze_forever!(e), same but wrapped in a loop.
   3. sub_scope!(e), that checks it is not at a lower level, checks that e is a ScopeProducer and then unwraps it and awaits it.

Would that work? I'm not sure we need to brand the timecapsule with a tag lifetime, because in this variant it remains implicit to the scope! and sub_scope! macro, so can't be snuggled.

I have an implementation in #11, but in a very rough state.

1. readme, docs, counterexamples to update
2. I would like to try porting the gist linked above
3. Naming gets confusing because I introduced yet another type called Scope (the result of the scope! macro)
4. That type should probably have a trait counterpart so that the signature of BoxScope::new become S: Scope rather than `Scope<??P??,

[steffahn]

3. sub_scope!(e), that checks it is not at a lower level, checks that e is a ScopeProducer and then unwraps it and awaits it.

Ah, wrap the whole producer and never expose the actual time_capsule variable, that might work, I’ll think about this (no earlier than) tomorrow.

[steffahn]

I would like to try porting the gist linked above

There’s likely going to be some “fun extra challenge” in the form of getting the impl Future to capture the lifetime of the &mut TimeCapsule when writing the signature of e.g. zip_files.

---

Oh… that doesn’t even work at all:

use core::future::Future;

struct Wrap<P>(P);
struct Capsule;

enum Never {}

fn f(x: String) -> Wrap<impl for<'a> FnOnce(&'a mut Capsule) -> (impl Future<Output = Never> + 'a)> {
    Wrap(|t: &mut _| async move {
        let _x = t;
        async {}.await;
        loop {}
    })
}

error[E0562]: `impl Trait` is not allowed in the return type of `Fn` trait bounds
 --> src/lib.rs:8:66
  |
8 | fn f(x: String) -> Wrap<impl for<'a> FnOnce(&'a mut Capsule) -> (impl Future<Output = Never> + 'a)> {
  |                                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `impl Trait` is only allowed in arguments and return types of functions and methods
  = note: see issue #99697 <https://github.com/rust-lang/rust/issues/99697> for more information
  = help: add `#![feature(impl_trait_in_fn_trait_return)]` to the crate attributes to enable
  = note: this compiler was built on 2024-03-02; consider upgrading it if it is out of date

For more information about this error, try `rustc --explain E0562`.
error: could not compile `playground` (lib) due to 1 previous error

and even with the nightly feature…

error: higher kinded lifetime bounds on nested opaque types are not supported yet
 --> src/lib.rs:9:96
  |
9 | fn f(x: String) -> Wrap<impl for<'a> FnOnce(&'a mut Capsule) -> (impl Future<Output = Never> + 'a)> {
  |                                                                                                ^^
  |
note: lifetime declared here
 --> src/lib.rs:9:34
  |
9 | fn f(x: String) -> Wrap<impl for<'a> FnOnce(&'a mut Capsule) -> (impl Future<Output = Never> + 'a)> {
  |                                  ^^

So either use dyn Future, or it’s probably better to try wrapping not the closure but just the future, and using lifetimes to enforce the connection.

[steffahn]

and even with the nightly feature…

Ah, the closure needn’t be generic over the lifetime, I suppose… so at least on nightly this actually compiles:

#![feature(impl_trait_in_fn_trait_return)]
use core::future::Future;

struct Wrap<P>(P);
struct Capsule;

enum Never {}

fn f<'a>(x: String) -> Wrap<impl FnOnce(&'a mut Capsule) -> (impl Future<Output = Never> + 'a)> {
    Wrap(|t| async move {
        let _x = t;
        async {}.await;
        loop {}
    })
}

but that doesn’t help stable much, still.