Memory error with repeated calls

[aekobear]

I wrote a small script to benchmark wasmoon's runtime overhead with multiple calls. However calling doString() more than a few dozen times causes a memory error

const { LuaFactory } = require('wasmoon')

async function init() {
  const factory = new LuaFactory();
  const lua = await factory.createEngine();
  
  const length = 1000;
  for (let i = 0; i < length; i++) {
    const a = Math.floor(Math.random() * 100);
    const b = Math.floor(Math.random() * 100);
    try {
      const result = await lua.doString(`return ${a} + ${b};`);
      console.log(result);
    } catch (error) {
      console.log(error);
    }
  }

  lua.global.close()
}

init();

This script should print 1000 random numbers then return. It always works for roughly the first 60, the crashes with:

RuntimeError: memory access out of bounds
    at <anonymous> (wasm://wasm/00109376:1:77112)
    at <anonymous> (wasm://wasm/00109376:1:71192)
    at <anonymous> (wasm://wasm/00109376:1:79616)
    at <anonymous> (wasm://wasm/00109376:1:94618)
    at <anonymous> (wasm://wasm/00109376:1:87842)
    at <anonymous> (wasm://wasm/00109376:1:63545)
    at nc (file:///home/eikobear/code/goodsheet/tests/node_modules/.deno/[email protected]/node_modules/wasmoon/dist/index.js:1375:366)
    at <anonymous> (wasm://wasm/00109376:1:55544)
    at <anonymous> (wasm://wasm/00109376:1:19806)
    at <anonymous> (wasm://wasm/00109376:1:30689)
RuntimeError: memory access out of bounds
    at <anonymous> (wasm://wasm/00109376:1:77112)
    at <anonymous> (wasm://wasm/00109376:1:73026)
    at <anonymous> (wasm://wasm/00109376:1:71202)
    at <anonymous> (wasm://wasm/00109376:1:79616)
    at <anonymous> (wasm://wasm/00109376:1:136610)
    at file:///home/eikobear/code/goodsheet/tests/node_modules/.deno/[email protected]/node_modules/wasmoon/dist/index.js:1268:365
    at Object.e.ccall (file:///home/eikobear/code/goodsheet/tests/node_modules/.deno/[email protected]/node_modules/wasmoon/dist/index.js:1376:346)
    at LuaWasm.pointersToBeFreed [as lua_newthread] (file:///home/eikobear/code/goodsheet/tests/node_modules/.deno/[email protected]/node_modules/wasmoon/dist/index.js:1626:49)
    at Global.newThread (file:///home/eikobear/code/goodsheet/tests/node_modules/.deno/[email protected]/node_modules/wasmoon/dist/index.js:92:38)
    at LuaEngine.callByteCode (file:///home/eikobear/code/goodsheet/tests/node_modules/.deno/[email protected]/node_modules/wasmoon/dist/index.js:1224:40)

I tested on [email protected] and [email protected] and got the same result

[gudzpoz]

According to the Lua manual:

When you interact with the Lua API, you are responsible for ensuring consistency. In particular, you are responsible for controlling stack overflow. You can use the function lua_checkstack to ensure that the stack has extra slots when pushing new elements.

Running return statements pushes values onto the stack. Without lua.global.pop() / lua.global.lua.lua_checkstack(lua.global.address), the stack grows and eventually overflows.

I am not sure whether this library should call lua_checkstack for the user though. But remembering to pop values off the stack should free you from random overflows.

[aekobear]

According to the Lua manual:

When you interact with the Lua API, you are responsible for ensuring consistency. In particular, you are responsible for controlling stack overflow. You can use the function lua_checkstack to ensure that the stack has extra slots when pushing new elements.

Running return statements pushes values onto the stack. Without lua.global.pop() / lua.global.lua.lua_checkstack(lua.global.address), the stack grows and eventually overflows.

I am not sure whether this library should call lua_checkstack for the user though. But remembering to pop values off the stack should free you from random overflows.

This helps me move forward, thank you!

I do think this is something to fix. When using the API directly, it makes sense to manage the stack because that's how you communicate with lua:

lua_call(L, 1, 1) // call a function
result = lua_pop(L, 1) // get its result

However in wasmoon, not only does doString already copy the value out of the stack, but the wasmoon wrapper for popping the stack doesn't actually return a value (which may also be a bug):

// i already have my result, so no incentive to access stack
const result = await lua.doString(`return ${a} + ${b};`);

// stackResult is always undefined. This is just busy-work
const stackResult = lua.global.pop();

[ceifa]

@aekobear thanks for the suggestion, I think it does make sense for wasmoon to pop the values returned, I can't think on a use case for returning the value and it continue on the stack

[ceifa]

@tims-bsquare do you have any opinion on this?

[tims-bsquare]

I think the lua_xmove is the memory leak in callByteCode. I can't remember why it's that way though. I have a suspicion something about returning a function from doString and calling it later but in theory a reference to that should continue to exist through lua_ref. Could try and remove it and see if things break?

[ceifa]

Could try and remove it and see if things break?

I'm not 100% confident in this solution, but the tests passed oO