quickjs-emscripten icon indicating copy to clipboard operation
quickjs-emscripten copied to clipboard

In context of `QuickJSAsyncRuntime` and `QuickJSAsyncContext`, the `callFunction()` not working as expected

Open mattvalleycodes opened this issue 1 year ago • 7 comments
trafficstars

Hi @justjake

First and foremost, thank you for the great work on the quickjs-emscripten. The following can be considered a question or a bug report. Please take it with a grain of salt. I've been building on top of the quickjs-emscripten package for some time, but I am still learning it, so I may be doing something wrong here! Now, I'd like to explain my scenario.

The ultimate goal is to execute the following codes without issue:

// Example A
test("hello world", function() {
  cnosole.log("hello, world!");
});

and

// Example B
test("hello world", function() {
  fetch("https://httpbin.org/ip");
});

Some facts:

  • Both codes are executed using QuickJSAsyncRuntime and QuickJSAsyncContext.
  • console.log, fetch and test are both APIs that are added to the context using the context.setProp API.
  • Example A works fine as it has no asynchronous code.
  • Example B errors with call_indirect to a null table entry (evaluating 'e.apply(null,arguments)' and I can see the Aborted(Assertion failed: list_empty(&rt->gc_obj_list), at: ../../vendor/quickjs/quickjs.c,1986,JS_FreeRuntime) in Dev Console.

This is the implementation of my test function:

  private test(...args: Array<QuickJSHandle>) {   
    const descriptionHandle = args[0];
    const description = this.context.getString(descriptionHandle);

    const callbackHandle = args[1];
    // FYI, this.context is just an instance of QuickJSAsyncContext created using QuickJSAsyncRuntime.newContext()
    this.context.callFunction(callbackHandle, this.context.undefined);
  }
  • Is it possible to call an async function (fetch in my example) inside a callback, wait for it to complete and execute the rest of the callback?
  • If it's possible, what's the recommended approach to handle a scenario like this.

Thanks!

mattvalleycodes avatar May 06 '24 00:05 mattvalleycodes

I don’t know want that would happen. How is fetch implemented?

justjake avatar May 06 '24 08:05 justjake

@justjake Thanks for looking into this issue. I simplified fetch yet getting the same here. This is how I'm defining the fetch function.

        const fetch = this.context.newAsyncifiedFunction("fetch", async () => {
          return this.context.newString("fetch returned.");
        });

        this.context.setProp(this.context.global, "fetch", fetch);

I'm curious: is this the correct way to call a callback that has an async operation in it?

this.context.callFunction(callbackHandle, this.context.undefined);

mattvalleycodes avatar May 06 '24 09:05 mattvalleycodes

And to clarify, the following works just fine (using newFunction instead of newAsyncifiedFunction)

        const fetch = this.context.newFunction("fetch", () => {
          return this.context.newString("fetch returned.");
        });

        this.context.setProp(this.context.global, "fetch", fetch);

mattvalleycodes avatar May 06 '24 09:05 mattvalleycodes

What should fetch return inside the VM, should it return a Promise<T> and be an async function from the perspective of other code in the VM, or should it be a synchronous function from the perspective of the code in the VM?

How are you executing your code in the VM? If it’s via callFunction, this won’t work, you need the asyncified variant if you’re calling asyncified code. You can’t call async code in a sync way from the host.

If it’s okay for fetch to be a normal async function inside the VM and return a Promise to its caller instead of a sync function, you should stop using “asyncified” APIs. The ASYNCIFY stuff is to make an async host function appear to be synchronous inside the VM. Is that your goal? Otherwise you can create a normal function, and return a promise handle.

justjake avatar May 07 '24 10:05 justjake

Thanks for coming back to me.

What should fetch return inside the VM, should it return a Promise and be an async function from the perspective of other code in the VM, or should it be a synchronous function from the perspective of the code in the VM?

It should be a synchronous function from the perspective of the code in the VM.

How are you executing your code in the VM? If it's via callFunction, this won't work, you need the asyncified variant if you're calling asyncified code. You can't call async code in a sync way from the host.

I'm using callFunction, but it's the asyncified variant. My context (this.context) is an instance of QuickJSAsyncContext

If it's okay for fetch to be a normal async function inside the VM and return a Promise to its caller instead of a sync function, you should stop using "asyncified" APIs. The ASYNCIFY stuff is to make an async host function appear to be synchronous inside the VM. Is that your goal? Otherwise you can create a normal function, and return a promise handle.

Thanks! We're intentionally using asyncified variants to make sure that the fetch API (async in the host) is used as it was synchronous. I will work on a simplified version of the issue and share it with you.

mattvalleycodes avatar May 08 '24 22:05 mattvalleycodes

You need to use evalCodeAsync, callFunction isn't async-aware.

justjake avatar May 09 '24 04:05 justjake

You need to use evalCodeAsync, callFunction isn't async-aware.

evalCodeAsync solves that problem but introduces an issue that callFunction didn't have it.

When I use evalCodeAsync to execute the following code, I get the 'name' is not defined error. It makes sense as evalCodeAsync is running the callback code in silo.

const name = "Teddy";

test("hello world", function() {
  console.log("name is ->', name);
  fetch("https://httpbin.org/ip");
});

Is there a way to fix this issue?

mattvalleycodes avatar May 12 '24 07:05 mattvalleycodes

Closing this one.

mattvalleycodes avatar Sep 18 '24 06:09 mattvalleycodes