Support added for exceptions with ASYNCIFY #2772
Conversation
| if (!___async_cur_frame) { | ||
| // print ugly integer in console, same as if an exception is thrown out of main() | ||
| if (activeException) _emscripten_async_abort_with_exception(); | ||
| return; |
There was a problem hiding this comment.
emscripten_async_resume is also used used by coroutine functions, if coroutine A creates and runs B, and there's an exception threw in B, I wonder how should A respond.
I've noticed that asyncify fails if you call alloca during an async callback, we should at least catch this and abort before corrupting the stack. Also added support for a third argument to realloc_ctx to refresh the invoke flag.
…ard against corruption when alloca is in use
This is a very helpful feature for asyncify! Using 'noExitRuntime' is a very blunt solution that means that the runtime is never shut down, but with this tweak, we clean up main normally when its async_cb exits.
| @@ -87,7 +87,10 @@ Module['callMain'] = Module.callMain = function callMain(args) { | |||
| #endif | |||
|
|
|||
| // if we're not running an evented main loop, it's time to exit | |||
| exit(ret); | |||
| #if ASYNCIFY | |||
| if (asm['getAsync'] === undefined || !asm['getAsync']()) | |||
There was a problem hiding this comment.
enough to do the second part. undefined is falsey.
There was a problem hiding this comment.
Sorry, it probably can't. I was thinking that it wouldn't get linked in if there were no async functions (emscripten_async_resume for example can get discarded), but getAsync is part of the Runtime interface so I think you're right, it's guaranteed not to be trimmed by any optimisation phase if ASYNCIFY is turned on.
There was a problem hiding this comment.
ok, then let's remove the unnecessary check here.
otherwise this looks ok to me, what do you think @coolwanglu ?
|
Wait, actually it's not OK! While writing some unittests, I've caught another problem with the stack unwinding when exceptions interact with ASYNCIFY. Some calls to I'm starting to think that explicitly restoring the stack in catch blocks (as I mused on the emscripten-discuss group) is going to be the only way to make the asyncify unwinding safe with exceptions... I need to thoroughly unittest this all before we can merge this pull request, sorry. It's not as straightforward as I thought! Pretty close though. |
|
Ok. As mentioned on the mailing list, explicitly restoring the stack in landingpads does sound like something we should do here. |
(tests commit 59e10a383f385edeedca1e01a2755d179058cbc1 in emscripten-fastcomp)
From inspecting the code through to creating the testcase took a while, but passing structs by val is the key to breaking asyncify this time.
kripken
force-pushed
the
incoming
branch
2 times, most recently
from
92ca007
to
c4f0934
Compare
|
Closing; ASYNCIFY is ancient and won't ever come back. We no longer use the code in this PR. |
|
Yeah, we should probably remove the option at this point, it's been deprecated for quite a while. Opened #5801 for that. |
The general scheme is not very complicated. Changes:
dynCall_vion the _foo___asnc_cb function, we useinvoke_vi. This is essential if the callback functions are to throw exceptions.emscripten_alloc_async_contextnow has an extra argument which the C++ code uses to indicate whether the async function is called with "invoke" or "call". (So theinvokeargument toemscripten_alloc_async_contextis set when__THREW__is checked immediately after the async call. We also update the invoke flag inemscripten_realloc_context.)main).This pull request works in conjunction with the pull request to the fastcomp compiler.