emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.91k stars 3.32k forks source link

Using module in environments without setTimeout, clearTimeout, atob causing memory leak #22979

Closed hellozyemlya closed 1 week ago

hellozyemlya commented 1 week ago

Generated js files provide stubs for that(and probably any other missing\stubbed) functions, and it puts it to global scope, capturing memory and wasm code instances, creating memory leaks.

That happens within audio worklet, for example, with this flags used for compilation:

set(EMSCRIPTEN_COMPILE_FLAGS "${EMSCRIPTEN_COMPILE_FLAGS} -O2")
set(EMSCRIPTEN_COMPILE_FLAGS "${EMSCRIPTEN_COMPILE_FLAGS} -mnontrapping-fptoint")
set(EMSCRIPTEN_COMPILE_FLAGS "${EMSCRIPTEN_LINK_FLAGS} -mnontrapping-fptoint")
set(EMSCRIPTEN_LINK_FLAGS "${EMSCRIPTEN_LINK_FLAGS} -s MODULARIZE=1")
set(EMSCRIPTEN_LINK_FLAGS "${EMSCRIPTEN_LINK_FLAGS} -s EXPORT_NAME=\"'lyrebird'\"")
set(EMSCRIPTEN_LINK_FLAGS "${EMSCRIPTEN_LINK_FLAGS} -s EXPORT_ES6")
set(EMSCRIPTEN_LINK_FLAGS "${EMSCRIPTEN_LINK_FLAGS} -s ENVIRONMENT=web,worker,shell")
set(EMSCRIPTEN_LINK_FLAGS "${EMSCRIPTEN_LINK_FLAGS} -s POLYFILL=1")
set(EMSCRIPTEN_LINK_FLAGS "${EMSCRIPTEN_LINK_FLAGS} -O2 --embind-emit-tsd hello.d.ts -s SINGLE_FILE")
hellozyemlya commented 1 week ago

As a workaround I adding globalThis.setTimeout, globalThis.clearTimeout and globalThis.atob stubs before calling actual module, that helps avoid leaks.

It seems like globalThis must not be involved at all to avoid such issues in future.

sbc100 commented 1 week ago

It looks like our polyfills for globalThis.clearTimeout and globalThis.atob cannot be capturing any state, so I guess it must be globalThis.setTimeout that is the issue: https://github.com/emscripten-core/emscripten/blob/eff9671ccb4a68c67c6d000a51f1426272103119/src/shell.js#L321-L322

However it looks like they are only supposed to be installed in the shell environment (such as spidermonkey) and not in an audio worklet.

It looks like you trying to target the audio worklet environment but without using the AUDIO_WORKLET setting? Is that right? I don't think we have any testing for that configuration. One possible fix is to make the above shell.js code explicitly only run if ENVIRONMENT_IS_SHELL.. however, given that it is already a very bespoke setup perhaps you existing workaround of adding stubs is good enough.

Finally, does the emscripten module not live as long as the audio worklet you are using? i.e. I wonder why it matters that you cannot GC the references to it if the lifetime of the audio worklet is the same as the emscripten module?