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

Awaiting multiple asynchronous calls in callback function #105

Open
rkristof opened this issue Jan 17, 2024 · 3 comments
Open

Awaiting multiple asynchronous calls in callback function #105

rkristof opened this issue Jan 17, 2024 · 3 comments

Comments

@rkristof
Copy link
Contributor

rkristof commented Jan 17, 2024

I'm using the workaround from the readme to allow using await in callback functions. It works well when there is only one asynchronous call inside the callback function. But if there are more, it will produce an infinite loop after the first call is finished. I managed to find a solution for this, so I thought I would post it here. Also I'm not very experienced with coroutines, so maybe you could tell me if my solution is acceptable.

Basically what I did was to extend the condition at the end of the local "step" function.

Before:

if safe and result == Promise.resolve(result) then
    result:finally(step)
else
    step()
end

After:

if safe and (result == Promise.resolve(result) or result ~= nil) then
    result:finally(step)
else
    step()
end
@rkristof rkristof changed the title Awaiting multiple asynchronous calls in callback method Awaiting multiple asynchronous calls in callback function Jan 17, 2024
@ceifa
Copy link
Owner

ceifa commented Mar 29, 2024

Here is how I use it: https://github.com/ceifa/demoon/blob/main/src/std.lua
It works great for me, do you have a minimal repro for this infinite loop?

@gudzpoz
Copy link
Contributor

gudzpoz commented Mar 30, 2024

I don't know if this is the same issue, but I managed to trigger a use-after-free issue when using multiple async calls grouped by Promise.all:

import { LuaFactory } from 'wasmoon';

const factory = new LuaFactory();
const L = await factory.createEngine();

const sleep = (ms) => new Promise((r) => setTimeout(r, ms));
L.global.set('sleep', sleep);

const results = await Promise.all([
  L.doString('sleep(400):await(); return 1'),
  sleep(100)
    .then(() => L.doString('sleep(500):await(); return 2')),
  sleep(200)
    .then(() => L.doString('sleep(3000):await(); return 3')),
  sleep(300)
    .then(() => L.doString('sleep(1000):await(); collectgarbage(); return 4')),
]);

console.log(results);

Basically, the threadIndex tracked does not guarantee to point to the same thread after the other thread manipulates the stack.

const threadIndex = this.global.getTop()

this.global.remove(threadIndex)

The code snippet above does the following:

  1. Creates four threads (Stack now: [T1, T2, T3, T4])
  2. T1 removes itself from stack (Stack now: [T2, T3, T4])
    • However, threadIndex tracked by T{N} is still N, which now points to T{N+1}.
  3. T2 ends and removes T3 from stack.
  4. T4 ensures to call collectgarbage(), which deletes T3 now that nowhere references it.
  5. T3 is freed but used by the return promise, causing a RuntimeError: memory access out of bounds.

Maybe luaL_ref should be used instead?

@rkristof
Copy link
Contributor Author

rkristof commented Apr 8, 2024

It seems I cannot reproduce the issue anymore. I thought it would be easy to reproduce, but I've done some testing where I combined multiple callback functions with asynchronous calls, and the original workaround from the readme worked fine every time. I guess my case was either very specific (which I cannot recreate now), or I made some other mistake somewhere.

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

No branches or pull requests

3 participants