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

Future support redesign. #153

Closed
wants to merge 2 commits into from
Closed

Future support redesign. #153

wants to merge 2 commits into from

Conversation

DelSkayn
Copy link
Owner

@DelSkayn DelSkayn commented May 26, 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 Jjavascript 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.
Ideally rquickjs would support asynchronous aware mutexes for locking the runtime.

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 Ctx is not valid for 'static' as it has the 'js` lifetime.

async fn read_file<'js>(ctx: Ctx<'js>, path: String) -> Result<String>{
    tokio::fs::read_to_string(&path).await
}

This severely limits the actual usefulness of the using futures with rquickjs.
Ideally futures would only have to be valid for the 'js lifetime.

Implementation

This PR is an attempt to address these problems.

First it introduces AsyncContext a version of Context which uses asynchronous locking instead of the blocking lock.
This version of context is used together with the async_with macro, which allows the with closure to return a future:

let rt = Runtime::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).

Currently futures are send to a separate task where the are polled in depended of whether the runtime is actually locked requiring the futures to be static.
This restriction is lifted by keeping any future spawned inside the async_with closure inside the runtime lock.

@DelSkayn
Copy link
Owner Author

DelSkayn commented May 26, 2023

Note:

The macro async_with is required because we need a way to create an invariant unique lifetime inside the future.
The following implementation doesn't work because the higher kinded lifetime causes the lifetime to only be valid for the duration of the closure and the future is thus not able to capture it.

async fn async_with<F,Fut,R>(f: F) -> R
where:
    F: for<'js> FnOnce(Ctx<'js>) -> Fut,
    Fut: Future<Output = R>,
{
    ...
}

Instead AsyncContex implements unsafe_async_with which has the following signiture:

unsafe async fn unsafe_async_with<F,Fut,R>(f: F) -> R
where:
    F: FnOnce(NonNull<qjs::JSContext>) -> Fut,
    Fut: Future<Output = R>,
{
    ...
}

The macro makes using this function safe.

@DelSkayn
Copy link
Owner Author

Closed in favor of #156

@DelSkayn DelSkayn closed this May 31, 2023
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.

None yet

1 participant