Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redesigned support for futures. #156

Merged
merged 22 commits into from Jun 5, 2023
Merged

Redesigned support for futures. #156

merged 22 commits into from Jun 5, 2023

Conversation

DelSkayn
Copy link
Owner

@DelSkayn DelSkayn commented May 31, 2023

Motivation

Rquickjs support mapping Javascript promises to futures and vice versa via the Promise and Promised types.
This allows the rquickjs library to interface with async rust.

However the current implementation has a few problems:

First, any access to the Javascript runtime in rquickjs has to go through a lock.
When the parallel feature is enabled this lock is done with a mutex.
Blocking locks are a big no-no in async rust, instead we should use a async-aware mutex.

Second, futures in rquickjs currently require that the future is valid for 'static.
This leads to a bit of a problem when trying to use rquickjs functions as basically all functionality has the 'js lifetime attached to it.
This prevents functions like the one below from being implemented as Function is not valid for 'static as it has the 'js lifetime.

async fn delay<'js>(amount: f64, cb: Function<'js>) -> Result<String>{
    tokio::time::sleep(Duration::from_secs_f64(amount)).await;
   cb.call::<(),()>(())
}

This severely limits the actual usefulness of the using futures with rquickjs.
Futures spawned inside the runtime should probably only require a lifetime of 'js.

Implementation

This PR is an attempt to address these problems.

First it introduces AsyncRuntime and AsyncContext, variants of runtime and context which use a async-aware mutex.
As a result almost all methods of these types are asynchronous.
This version of context is used together with the async_with macro, which allows the with closure to return a future:

let rt = AsyncRuntime::new().unwrap();

let ctx = AsyncContext::full(&rt).await.unwrap();

async fn delay<'js>(ctx: Ctx<'js>, amount: f64, cb: Function<'js>) -> Result<()> {
    tokio::time::sleep(Duration::from_secs_f64(amount)).await;
    cb.call::<(), ()>(())
}

fn print(text: String) {
    println!("{}", text)
}

async_with!(ctx => |ctx|{
    let f = Func::from(Async(delay));

    ctx.globals().set("print", Func::from(print)).unwrap();
    ctx.globals().set("delay", f).unwrap();
    let promise = ctx.eval::<Function,_>("async () => { await delay(1,() => print('delayed')); }")
    .unwrap();

    promise.call::<(),Promise<()>>(()).unwrap().await;
})
.await;

Second it lifts the 'static requirement for futures used in Promise and Promised.
This allows futures to keep Javascript objects, as in the above example, allowing one to implement for example the delay function much easier than is currently possible (see #128).

Finally it makes the library completely async runtime agnostic, all features are just implemented on top of the standard library future types and thus the library no longer needs to depend on tokio, async-std or smol and the rquickjs crate features implementing functionality for those runtimes have been removed.

@DelSkayn
Copy link
Owner Author

DelSkayn commented Jun 2, 2023

I have encountered some problems when trying to use closures which return futures.
It seems currently impossible to create a closure with a signiture similar to the following:

let closure = |ctx: Ctx| async move {
    // Use ctx
    std::mem::drop(ctx);
};

While the following equivalent functions works fine.

async fn f|ctx: Ctx|{
    // Use ctx
    std::mem::drop(ctx);
};

Playground link
In order to create a closure which has similar semantics as the function above we need higher order closures but this still needs to be implemented (see rust-lang/rust#97362).

@DelSkayn DelSkayn merged commit 4b87dff into master Jun 5, 2023
50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test promise::test::async_std_tests::delayed_fn_single seems to deadlock Graceful shutdown of async executor
1 participant