ceifa / wasmoon

A real lua 5.4 VM with JS bindings made with webassembly
MIT License
462 stars 27 forks source link

RuntimeError: memory access out of bounds #62

Closed Drumsin closed 1 year ago

Drumsin commented 1 year ago

My project and number of Lua scripts has increased to where I get a:

RuntimeError: memory access out of bounds

Found that excluding any one of the larger scripts the error goes away, so it's definitely the length and project size.

Screenshot from 2022-11-07 11-48-07

Is there a way I can increase this and host my own glue.wasm? Currently it's making the request to: https://unpkg.com/wasmoon@1.14.0/dist/glue.wasm

Drumsin commented 1 year ago

I've looked through the lua source. I'm perfectly capable of doing this myself in creating a new build. I just don't know where to look, and what to change. Do you know?

It looks like you've helped someone out with some instructions on running a wasm build in these support threads. This is no issue for me. What I need to know is where and what do I change to increase the memory limit?

Thank you for your work,

A "sponsor" @ceifa

ceifa commented 1 year ago

Hey Drumsin, There is some "layers" where this memory limit may be coming from, it can be from emscripten(my first guess), lua itself, or the wasm engine. It's not an easy issue, that is why I didn't take a look yet.

And yes, you can host your own glue.wasm, my idea was to make a PR on your project with it, but didn't have time for this too. You can take a look at my example project for an example of how to do it: https://github.com/ceifa/try-lua

jfmmm commented 1 year ago

Would love to know how to do that too if you find how. Just managed to get wasmoon to load the Factorio lua files inside Electron and I get this error when trying to return the whole thing. I haven't even started loading mod on top of it too lol

I'll see if I can create an example repo of it, factorio lua data is public on github so it would be easy.

ceifa commented 1 year ago

There was a bug on chrome around the memory growth: https://bugs.chromium.org/p/v8/issues/detail?id=11863#c44 Which chrome/v8 version are you guys on?

@jfmmm nice, a minimal repro would be great

jfmmm commented 1 year ago

It's definitely a browser issue, I tried a minimal repro with a node.js script instead of electron and it work fine.

My electron environment is:

node: v16.16.0
v8: v10.6.194.23-electron.0
uv: v1.43.0
zlib: v1.2.12.1-motley
brotli: v1.0.9
ares: v1.18.1
modules: v109
nghttp2: v1.47.0
napi: v8
llhttp: v6.0.7
openssl: v1.1.1
cldr: v41.0
icu: v71.1
tz: v2022e
unicode: v14.0
electron: v21.2.2
chrome: v106.0.5249.168
jfmmm commented 1 year ago

I take that back, it does it with node.js too, just need to run it a few time then it start throwing it pretty consistently after about 4-6 run.

Here the repro -> https://github.com/jfmmm/wasmoon-factorio-example

ceifa commented 1 year ago

Apparently this only occurs in asynchronous APIs, for a quick workaround, I recommend using the synchronous ones

Drumsin commented 1 year ago

Apparently this only occurs in asynchronous APIs, for a quick workaround, I recommend using the synchronous ones

Not sure if this applies to my situation, or what exactly you mean by this?

I've tested with Firefox 106.0.2 (64-bit) Ubuntu with a RuntimeError, different error message.

RuntimeError: index out of bounds

Uncaught (in promise) RuntimeError: index out of bounds
    _lua_close build.js?fc66aab8d46b2bfeb407:2
    close build.js?fc66aab8d46b2bfeb407:2
    f build.js?fc66aab8d46b2bfeb407:2
    async* build.js?fc66aab8d46b2bfeb407:2
    EventListener.handleEvent* build.js?fc66aab8d46b2bfeb407:2
    <anonymous> build.js?fc66aab8d46b2bfeb407:2
    <anonymous> build.js?fc66aab8d46b2bfeb407:2

Tested with latest Chrome Version 107.0.5304.110 (Official Build) (64-bit) Ubunutu, gives:

RuntimeError: memory access out of bounds

ceifa commented 1 year ago

Not sure if this applies to my situation, or what exactly you mean by this?

I'm not sure too, but it worked for the minimal repro @jfmmm posted. What I mean with "use the sync ones instead of the async" is to use the "doStringSync" instead of "doString":

// Your current code:
// await lua.doString(luaCode)
// New synchronous code:
lua.doStringSync(luaCode)
Drumsin commented 1 year ago

Hey Ceifa,

First, thanks for your responses and help with this. It's much appreciated.

I've tried the mentioned lua.doStringSync, but still the same results.

// Your current code:
// await lua.doString(luaCode)
// New synchronous code:
lua.doStringSync(luaCode)

Prior, I kept an open mind that it might be an issue with the Lua code, but now I'm 100% confident it's length related.

Lua String Length Testing

All values below are string lengths before lua.doStringSync(luaCode)

Works

lua library game code: 607453 editor code: 977

Breaks

RuntimeError: memory access out of bounds

lua library game code: 607453 editor code: 76810

Drumsin commented 1 year ago

UPDATE: I decided to try this Lua minifier: https://www.npmjs.com/package/luamin Nearly cut the string length in half, and it's working now!

jfmmm commented 1 year ago

I guess we have a somewhat different issue. I was getting the issue when transferring data back from lua to my JS process. While you're having it while trying to bootstrap the thing with a large string.

Weirdly before @ceifa pointed out that the synchronous api worked, my workaround was to convert the data into a large 13mb string of json on the LUA side and passing it that way. It didn't gave me any error even with the async method.

jfmmm commented 1 year ago

@ceifa You could try in my example to pass the data in json format from LUA to JS and then use that same data and try sending it back to LUA as a JSON string. I bet it would repro his issue.

Here the library I was using (use JSON:encode, JSON:encode_pretty crash) http://regex.info/blog/lua/json

jude90 commented 1 year ago

image I came across a RangeError: WebAssembly.Instance(): Out of memory: wasm memory. when I call js function which returns promise and then await it in a for loop . OOM happened when function was invoked over 400 times

update: I fixed this problem by changing my functions into sync style

ceifa commented 1 year ago

@Drumsin I tried cloning your repo and commenting the minifier function but didn't help to reproduce the issue. Do I need to do something to get the error?

Drumsin commented 1 year ago

@Drumsin I tried cloning your repo and commenting the minifier function but didn't help to reproduce the issue. Do I need to do something to get the error?

@ceifa yes, you would have to own a copy of the game in order to extract the licensed Lua files and run a new webpack build. At aoe4.app and on GitHub repo, I don't incorporate the game library files for legal reasons. Legally I cannot provide them to you, but I'd be willing to gift you a Christmas copy of the game so you can access them. See: https://github.com/Drumsin/aoe4-generated-map-debugger#using-map-functions-debug-entire-maps

P.S. Thanks for the PR on running the wasm inline. I'll incorporate this once I get back to my dev machine. Brilliant!

ceifa commented 1 year ago

By coincidence I bought AoE4 yesterday! I will follow the instructions and try to reproduce the error, thanks!

Uigeadailzor commented 1 year ago

... Just managed to get wasmoon to load the Factorio lua files inside Electron and I get this error when trying to return the whole thing. ...

Small world! I've been doing the same (in browser, not Electron) and now get the same error.


I have also been getting this memory access out of bounds at glue.wasm error. It occurs when trying to extract data using luaEngine.global.get("...") after a substantial amount of lua scripts have run using both async doFile and async doString. I am using both Chrome and Firefox on Windows 10, and the same error occurs in both. The error didn't show up initially, it was not until after I had repeatedly run and tested various scripts (Factorio mods) many times that the error started showing up. I cannot reliably reproduce the error, restarting my machine or browser seems to fix it temporarily, but sometimes it doesn't. I initially thought it may be due to sheer size of the data I am trying to extract but the memory usage doesn't exceed ~300MB, and the first time I extracted data I produced ~50MB of JSON without issue. The exact same scenario now no longer works.

As a workaround I was able to print() a serialized version of the data using serpent - I'm sure a lua to JSON tool will work as well.

TL;DR: Switching all mountFile, doString, doFile to their synchronous versions seems to have fixed the issue for me (it might not have been necessary to change to mountFileSync but I changed it anyway). However I made this change after seeing it mentioned earlier in this thread and have only run it a few times, so it hasn't had much testing.

ceifa commented 1 year ago

This is a stack limit problem, increasing the stack size to 10MB fixes the problem, but I'm not sure it is the best solution. A possible solution is storing the string on heap depending on the size of it.

Waiting for https://github.com/emscripten-core/emscripten/issues/18401 to continue working on this issue

ceifa commented 1 year ago

The original @Drumsin issue was fixed in #73. I'm not sure about the other issues @Uigeadailzor @jude90 and @jfmmm pointed, please open a new issue if persisted.