C++ exceptions caught in JavaScript leak stack memory even when documented instructions are followed about emscripten

[aheejin]

That sounds good. Can we close this then? Please feel free to reopen it if you have any remaining concerns. Thanks!

[sbc100]

Regarding the memory leak issue specifically, I don't think that the incrementExceptionRefcount/decrementExceptionRefcount calls are going to recover you stack memory. For one thing, I believe these function deal with heap allocations, not stack allocations, and also there could be any number of other data structure that are on the stack when the exception is thrown.

When you catch a native exception from JS I think its incumbent upon you to restore the stack pointer before calling back into Wasm. You can do this by calling var sp = stackSave() before the try and then stackRestore(sp) in the catch block. @aheejin correct me if I'm wrong here.

[aheejin]

So it looks the exception is allocated on the stack first, and then copied when thrown, because it is thrown by value. And what JS catches is that copied exception. So increment/decrementExceptionRefcount takes care of freeing the caught (=copied) exception, but not the original stack-allocated one. I think, as @sbc100 suggested, you should use stackSave and stackRestore to restore the stack, like this: #17313 (comment) (Nevermind the other stuff; only the uses of the two methods are relevant here.)
And we should probably consider adding that to the documentation, because it is easy to miss.

Also test_EXPORT_EXCEPTION_HANDLING_HELPERS is being tested both for Emscripten EH and Wasm EH, so your error is strange. It's hard to reproduce the error by reading your build script. Can you give me a command line reproducer and the source files that produces the export error?

[dbecroft]

Thanks very much for your responses! Manipulating the stack pointer is a game I'd rather not play, so I decided to add some C++ code to my Emscripten bindings to catch all exceptions before they exit WebAssembly and instead convert them into normal return values as { result } or { error } objects for the JavaScript caller. This change has resolved my memory leak.