Add ctx.poll

[richarddd]

This allows for us to block until a task is completed and drive spanwed tasks forward, for example:

globals.set(
    "blockUntilComplete",
    Func::from(move |ctx, promise| {
        struct Args<'js>(Ctx<'js>, Promise<'js>);
        let Args(ctx, promise) = Args(ctx, promise);
        let mut fut = pin!(promise.into_future::<Value>());
        task::block_in_place(move || {
            Handle::current().block_on(async move {
                poll_fn(move |cx| {
                    if let Poll::Ready(x) = fut.as_mut().poll(cx) {
                        return Poll::Ready(x);
                    }
                    ctx.poll(cx);
                    cx.waker().wake_by_ref();
                    Poll::Pending
                })
                .await
            })
        })
))?;

Ideally this should be moved to finish and somehow don't require task::block_in_place to access task Context. Can we store the current waker in the Opaque and use Context::from_waker? Feedback is welcome!

With this approach i'm able to run a very complex promise + timeout + sync TLA import test case:

async function main() {
  console.log("before promise");
  await new Promise((res) => setTimeout(res, 1000));
  console.log("after promise");

  blockUntilComplete(new Promise((res) => setTimeoutSpawn(res, 1000)));

  setTimeout(() => {
    console.log("nested setTimeout 1");
    setTimeout(async () => {
      console.log("nested setTimeout 2");

      await new Promise((res) => setTimeoutSpawn(res, 1000));
      console.log("awaited setTimeout 3");

      blockUntilComplete(new Promise((res) => setTimeout(res, 1000)));
      blockUntilComplete(import("./import.mjs"));

      console.log("blocking");
    }, 1000);
  }, 1000);
}

await main();

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 70.70707% with 29 lines in your changes are missing coverage. Please review.

Project coverage is 68.45%. Comparing base (613b82a) to head (8b88beb).
Report is 5 commits behind head on master.

Files	Patch %	Lines
core/src/context/ctx.rs	0.00%	17 Missing ⚠️
core/src/runtime/async.rs	85.36%	12 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #310      +/-   ##
==========================================
+ Coverage   68.31%   68.45%   +0.13%     
==========================================
  Files          83       83              
  Lines       12237    12336      +99     
==========================================
+ Hits         8360     8444      +84     
- Misses       3877     3892      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[richarddd]

@DelSkayn please take a look at this when time permits. If you don't like this approach maybe spawner can be accessible from ctx so this can be implemented outside the crate. Or do you perhaps have a better suggestion? 🙂

[richarddd]

Can be solved if this is merged:
#312

[DelSkayn]

I am somewhat hesitant with merging this as I am not sure this method is right for the library. The problem I see is poll as a function somewhat breaks the general design of futures.

rquickjs's approach to futures is to ensure that at some point execution should yield back to the executor driving the futures, yielding back through QuickJS out of the with closure. This is inline with how rust futures generally work and is done with tokio's budget system in mind. Tokio will occasionally have a future return Poll::Pending and immediately call the waker until all the futures leading up to that future return back to the executor so it might schedule another task when required and prevent certain long running tasks from blocking another.

This poll method doesn't work nicely with tokio budgets. Looking at the function one would assume that if you have a future and you call poll often enough it will eventually finish the future. But if tokio is trying to force a future to yield this will not be the case.

In your case you are able to work around it with task::block_in_place and then a block on. But I don't think this is something tokio would recommend doing.

So I am not sure it is a good idea to add a method designed to work around yielding back to the executor.

I think it might be useful to have more access to the rquickjs internal schedular but I think it should be a lower level API then what is in this PR. That way you can implement the functionality you need but rquickjs doesn't explicitly support a use case which is a bit of a hack.

That lower level API would then have a safe method for the schedular.poll and would return SchedularPoll so that users can determine what to do with it and a method to listen to new tasks being added.

[richarddd]

Thanks for your comment, I agree. Maybe you can expose spawner from Ctx so users can handle this themself while you decide on a better way forward? I’ll close the PR.