From 7ec6306bf30d19dc37aab7314f8e2f7527cfef95 Mon Sep 17 00:00:00 2001 From: Davit Samvelyan Date: Fri, 7 Feb 2025 12:31:58 +0100 Subject: [PATCH 1/8] Propagate errors through val call hierarchy. Fix bug when failure or exception happening in a >1 level of the val coroutine call hierarchy is not being propagated. As a result only deep most level coroutine promise is rejected, other val promises including top most one remain in a perpetual pending state. --- src/lib/libemval.js | 40 ++++++++++++++++----------------- src/lib/libsigs.js | 1 + system/include/emscripten/val.h | 32 +++++++++++++++++++++----- system/lib/embind/bind.cpp | 4 ++++ tools/emscripten.py | 1 + 5 files changed, 52 insertions(+), 26 deletions(-) diff --git a/src/lib/libemval.js b/src/lib/libemval.js index a9dbc4b72b1af..aa135b5051164 100644 --- a/src/lib/libemval.js +++ b/src/lib/libemval.js @@ -467,33 +467,33 @@ var LibraryEmVal = { return result.done ? 0 : Emval.toHandle(result.value); }, - _emval_coro_suspend__deps: ['$Emval', '_emval_coro_resume'], - _emval_coro_suspend: async (promiseHandle, awaiterPtr) => { - var result = await Emval.toValue(promiseHandle); - __emval_coro_resume(awaiterPtr, Emval.toHandle(result)); + _emval_coro_suspend__deps: ['$Emval', '_emval_coro_resume', '_emval_coro_reject'], + _emval_coro_suspend: (promiseHandle, awaiterPtr) => { + Emval.toValue(promiseHandle) + .then(result => __emval_coro_resume(awaiterPtr, Emval.toHandle(result))) + .catch(error => __emval_coro_reject(awaiterPtr, Emval.toHandle(error))); }, - _emval_coro_make_promise__deps: ['$Emval', '__cxa_rethrow'], + _emval_coro_make_promise__deps: ['$Emval'], _emval_coro_make_promise: (resolveHandlePtr, rejectHandlePtr) => { return Emval.toHandle(new Promise((resolve, reject) => { - const rejectWithCurrentException = () => { - try { - // Use __cxa_rethrow which already has mechanism for generating - // user-friendly error message and stacktrace from C++ exception - // if EXCEPTION_STACK_TRACES is enabled and numeric exception - // with metadata optimised out otherwise. - ___cxa_rethrow(); - } catch (e) { - // But catch it so that it rejects the promise instead of throwing - // in an unpredictable place during async execution. - reject(e); - } - }; - {{{ makeSetValue('resolveHandlePtr', '0', 'Emval.toHandle(resolve)', '*') }}}; - {{{ makeSetValue('rejectHandlePtr', '0', 'Emval.toHandle(rejectWithCurrentException)', '*') }}}; + {{{ makeSetValue('rejectHandlePtr', '0', 'Emval.toHandle(reject)', '*') }}}; })); }, + + _emval_coro_exception_to_error__deps: ['$Emval', '__cxa_rethrow'], + _emval_coro_exception_to_error: () => { + try { + // Use __cxa_rethrow which already has mechanism for generating + // user-friendly error message and stacktrace from C++ exception + // if EXCEPTION_STACK_TRACES is enabled and numeric exception + // with metadata optimised out otherwise. + ___cxa_rethrow(); + } catch (e) { + return Emval.toHandle(e); + } + }, }; addToLibrary(LibraryEmVal); diff --git a/src/lib/libsigs.js b/src/lib/libsigs.js index c5474f4847718..33b4bf642b3e3 100644 --- a/src/lib/libsigs.js +++ b/src/lib/libsigs.js @@ -339,6 +339,7 @@ sigs = { _emval_await__sig: 'pp', _emval_call__sig: 'dpppp', _emval_call_method__sig: 'dppppp', + _emval_coro_exception_to_error__sig: 'p', _emval_coro_make_promise__sig: 'ppp', _emval_coro_suspend__sig: 'vpp', _emval_decref__sig: 'vp', diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h index 0e0e511cb64df..3ba0d4d639b6c 100644 --- a/system/include/emscripten/val.h +++ b/system/include/emscripten/val.h @@ -24,7 +24,6 @@ #include #endif - namespace emscripten { class val; @@ -118,6 +117,7 @@ EM_VAL _emval_iter_next(EM_VAL iterator); #if __cplusplus >= 202002L void _emval_coro_suspend(EM_VAL promise, void* coro_ptr); +EM_VAL _emval_coro_exception_to_error(); EM_VAL _emval_coro_make_promise(EM_VAL *resolve, EM_VAL *reject); #endif @@ -712,11 +712,13 @@ class val::awaiter { // - initially created with promise // - waiting with a given coroutine handle // - completed with a result - std::variant, val> state; + // - rejected with an error + std::variant, val, val> state; constexpr static std::size_t STATE_PROMISE = 0; constexpr static std::size_t STATE_CORO = 1; constexpr static std::size_t STATE_RESULT = 2; + constexpr static std::size_t STATE_ERROR = 3; public: awaiter(const val& promise) @@ -743,9 +745,20 @@ class val::awaiter { coro.resume(); } + void reject_with(val&& error) { + auto coro = std::move(std::get(state)); + state.emplace(std::move(error)); + coro.resume(); + } + // `await_resume` finalizes the awaiter and should return the result // of the `co_await ...` expression - in our case, the stored value. - val await_resume() { return std::move(std::get(state)); } + val await_resume() { + if (state.index() == STATE_ERROR) { + throw std::get(state); + } + return std::move(std::get(state)); + } }; inline val::awaiter val::operator co_await() const { @@ -756,7 +769,7 @@ inline val::awaiter val::operator co_await() const { // that compiler uses to drive the coroutine itself // (`T::promise_type` is used for any coroutine with declared return type `T`). class val::promise_type { - val promise, resolve, reject_with_current_exception; + val promise, resolve, reject; public: // Create a `new Promise` and store it alongside the `resolve` and `reject` @@ -766,7 +779,7 @@ class val::promise_type { EM_VAL reject_handle; promise = val(internal::_emval_coro_make_promise(&resolve_handle, &reject_handle)); resolve = val(resolve_handle); - reject_with_current_exception = val(reject_handle); + reject = val(reject_handle); } // Return the stored promise as the actual return value of the coroutine. @@ -779,7 +792,14 @@ class val::promise_type { // On an unhandled exception, reject the stored promise instead of throwing // it asynchronously where it can't be handled. void unhandled_exception() { - reject_with_current_exception(); + try { + std::rethrow_exception(std::current_exception()); + } catch (const val& error) { + reject(error); + } catch (...) { + val error = val(internal::_emval_coro_exception_to_error()); + reject(error); + } } // Resolve the stored promise on `co_return value`. diff --git a/system/lib/embind/bind.cpp b/system/lib/embind/bind.cpp index fa32bd173ca4f..c9f1a18621e88 100644 --- a/system/lib/embind/bind.cpp +++ b/system/lib/embind/bind.cpp @@ -67,6 +67,10 @@ void _emval_coro_resume(val::awaiter* awaiter, EM_VAL result) { awaiter->resume_with(val::take_ownership(result)); } +void _emval_coro_reject(val::awaiter* awaiter, EM_VAL error) { + awaiter->reject_with(val::take_ownership(error)); +} + } namespace { diff --git a/tools/emscripten.py b/tools/emscripten.py index 52aefde8c6c07..5bc174acdd10e 100644 --- a/tools/emscripten.py +++ b/tools/emscripten.py @@ -1093,6 +1093,7 @@ def create_pointer_conversion_wrappers(metadata): 'emscripten_proxy_finish': '_p', 'emscripten_proxy_execute_queue': '_p', '_emval_coro_resume': '_pp', + '_emval_coro_reject': '_pp', 'emscripten_main_runtime_thread_id': 'p', '_emscripten_set_offscreencanvas_size_on_thread': '_pp__', 'fileno': '_p', From 84ec021ce961caae2f98f752d4e4eba228b0ffc4 Mon Sep 17 00:00:00 2001 From: Davit Samvelyan Date: Thu, 13 Feb 2025 19:10:41 +0100 Subject: [PATCH 2/8] Add tests for coro error/exception propogation. --- test/embind/test_val_coro.cpp | 60 +++++++++++++++++++++++++++++++++-- test/test_core.py | 21 +++++++++++- 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/test/embind/test_val_coro.cpp b/test/embind/test_val_coro.cpp index 442667e2a56cd..0482e716a73f8 100644 --- a/test/embind/test_val_coro.cpp +++ b/test/embind/test_val_coro.cpp @@ -16,10 +16,24 @@ EM_JS(EM_VAL, promise_sleep_impl, (int ms, int result), { return handle; }); +EM_JS(EM_VAL, promise_fail_impl, (), { + let promise = new Promise((_, reject) => setTimeout(reject, 1, new Error("bang from JS promise!"))); + let handle = Emval.toHandle(promise); + // FIXME. See https://github.com/emscripten-core/emscripten/issues/16975. +#if __wasm64__ + handle = BigInt(handle); +#endif + return handle; +}); + val promise_sleep(int ms, int result = 0) { return val::take_ownership(promise_sleep_impl(ms, result)); } +val promise_fail() { + return val::take_ownership(promise_fail_impl()); +} + // Test that we can subclass and make custom awaitable types. template class typed_promise: public val { @@ -37,7 +51,13 @@ class typed_promise: public val { } }; +template val asyncCoro() { + co_return co_await asyncCoro(); +} + +template <> +val asyncCoro<0>() { // check that just sleeping works co_await promise_sleep(1); // check that sleeping and receiving value works @@ -50,12 +70,48 @@ val asyncCoro() { co_return 34; } +template val throwingCoro() { + co_await throwingCoro(); + co_return 56; +} + +template <> +val throwingCoro<0>() { throw std::runtime_error("bang from throwingCoro!"); co_return 56; } +template +val failingPromise() { + co_await failingPromise(); + co_return 65; +} + +template <> +val failingPromise<0>() { + co_await promise_fail(); + co_return 65; +} + +val caughtException() { + int result = 0; + try { + co_return co_await throwingCoro<2>(); + } catch (const val&) { // runtime_error turns into emscripten::val + result += 21; + } + try { + co_return co_await failingPromise<2>(); + } catch (const val&) { + result += 21; + } + co_return result; +} + EMSCRIPTEN_BINDINGS(test_val_coro) { - function("asyncCoro", asyncCoro); - function("throwingCoro", throwingCoro); + function("asyncCoro", asyncCoro<3>); + function("throwingCoro", throwingCoro<3>); + function("failingPromise", failingPromise<3>); + function("caughtException", caughtException); } diff --git a/test/test_core.py b/test/test_core.py index 6d24209be1727..b570c6d68f253 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -7473,7 +7473,7 @@ def test_embind_val_coro(self): self.emcc_args += ['-std=c++20', '--bind', '--post-js=post.js'] self.do_runf('embind/test_val_coro.cpp', '34\n') - def test_embind_val_coro_caught(self): + def test_embind_val_coro_propogate_cpp_exception(self): self.set_setting('EXCEPTION_STACK_TRACES') create_file('post.js', r'''Module.onRuntimeInitialized = () => { Module.throwingCoro().then( @@ -7484,6 +7484,25 @@ def test_embind_val_coro_caught(self): self.emcc_args += ['-std=c++20', '--bind', '--post-js=post.js', '-fexceptions'] self.do_runf('embind/test_val_coro.cpp', 'rejected with: std::runtime_error: bang from throwingCoro!\n') + def test_embind_val_coro_propogate_js_error(self): + self.set_setting('EXCEPTION_STACK_TRACES') + create_file('post.js', r'''Module.onRuntimeInitialized = () => { + Module.failingPromise().then( + console.log, + err => console.error(`rejected with: ${err.message}`) + ); + }''') + self.emcc_args += ['-std=c++20', '--bind', '--post-js=post.js', '-fexceptions'] + self.do_runf('embind/test_val_coro.cpp', 'rejected with: bang from JS promise!\n') + + def test_embind_val_coro_caught_exception(self): + self.set_setting('EXCEPTION_STACK_TRACES') + create_file('post.js', r'''Module.onRuntimeInitialized = () => { + Module.caughtException().then(console.log); + }''') + self.emcc_args += ['-std=c++20', '--bind', '--post-js=post.js', '-fexceptions'] + self.do_runf('embind/test_val_coro.cpp', '42\n') + def test_embind_dynamic_initialization(self): self.emcc_args += ['-lembind'] self.do_run_in_out_file_test('embind/test_dynamic_initialization.cpp') From 21d9d42792cc713eb3fb1346c2732eede3173dd0 Mon Sep 17 00:00:00 2001 From: Davit Samvelyan Date: Fri, 14 Feb 2025 10:23:43 +0100 Subject: [PATCH 3/8] Add missing include for exception handling. --- system/include/emscripten/val.h | 1 + 1 file changed, 1 insertion(+) diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h index 3ba0d4d639b6c..9752063be857d 100644 --- a/system/include/emscripten/val.h +++ b/system/include/emscripten/val.h @@ -21,6 +21,7 @@ #include #if __cplusplus >= 202002L #include +#include #include #endif From ee390ddaed41d17da9fd4b2145b235512a54bee7 Mon Sep 17 00:00:00 2001 From: Davit Samvelyan Date: Mon, 17 Feb 2025 16:19:31 +0100 Subject: [PATCH 4/8] Propagate via rejections - Propagate errors via promise rejections rather then exceptions to ensure consistent behaviour between normal and coroutine functions. --- system/include/emscripten/val.h | 32 ++++++++++++++++++++------------ test/embind/test_val_coro.cpp | 16 ---------------- test/test_core.py | 8 -------- 3 files changed, 20 insertions(+), 36 deletions(-) diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h index 9752063be857d..b080cb0057983 100644 --- a/system/include/emscripten/val.h +++ b/system/include/emscripten/val.h @@ -713,13 +713,11 @@ class val::awaiter { // - initially created with promise // - waiting with a given coroutine handle // - completed with a result - // - rejected with an error - std::variant, val, val> state; + std::variant, val> state; constexpr static std::size_t STATE_PROMISE = 0; constexpr static std::size_t STATE_CORO = 1; constexpr static std::size_t STATE_RESULT = 2; - constexpr static std::size_t STATE_ERROR = 3; public: awaiter(const val& promise) @@ -732,7 +730,8 @@ class val::awaiter { bool await_ready() { return false; } // On suspend, store the coroutine handle and invoke a helper that will do - // a rough equivalent of `promise.then(value => this.resume_with(value))`. + // a rough equivalent of + // `promise.then(value => this.resume_with(value)).catch(error => this.reject_with(error))`. void await_suspend(std::coroutine_handle handle) { internal::_emval_coro_suspend(std::get(state).as_handle(), this); state.emplace(handle); @@ -746,18 +745,14 @@ class val::awaiter { coro.resume(); } - void reject_with(val&& error) { - auto coro = std::move(std::get(state)); - state.emplace(std::move(error)); - coro.resume(); - } + // When JS invokes `reject_with` with some error value, reject currently suspended + // coroutine's promise with the error value and destroy coroutine frame, because + // in this scenario coroutine never reaches final_suspend point to be destroyed automatically. + void reject_with(val&& error); // `await_resume` finalizes the awaiter and should return the result // of the `co_await ...` expression - in our case, the stored value. val await_resume() { - if (state.index() == STATE_ERROR) { - throw std::get(state); - } return std::move(std::get(state)); } }; @@ -803,12 +798,25 @@ class val::promise_type { } } + // Reject the stored promise due to rejection deeper in the call chain + void reject_with(val&& error) { + reject(std::move(error)); + } + // Resolve the stored promise on `co_return value`. template void return_value(T&& value) { resolve(std::forward(value)); } }; + +inline void val::awaiter::reject_with(val&& error) { + auto coro = std::move(std::get(state)); + auto& promise = coro.promise(); + promise.reject_with(std::move(error)); + coro.destroy(); +} + #endif // Declare a custom type that can be used in conjunction with diff --git a/test/embind/test_val_coro.cpp b/test/embind/test_val_coro.cpp index 0482e716a73f8..c962bd7931abe 100644 --- a/test/embind/test_val_coro.cpp +++ b/test/embind/test_val_coro.cpp @@ -94,24 +94,8 @@ val failingPromise<0>() { co_return 65; } -val caughtException() { - int result = 0; - try { - co_return co_await throwingCoro<2>(); - } catch (const val&) { // runtime_error turns into emscripten::val - result += 21; - } - try { - co_return co_await failingPromise<2>(); - } catch (const val&) { - result += 21; - } - co_return result; -} - EMSCRIPTEN_BINDINGS(test_val_coro) { function("asyncCoro", asyncCoro<3>); function("throwingCoro", throwingCoro<3>); function("failingPromise", failingPromise<3>); - function("caughtException", caughtException); } diff --git a/test/test_core.py b/test/test_core.py index b570c6d68f253..214bb517fec89 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -7495,14 +7495,6 @@ def test_embind_val_coro_propogate_js_error(self): self.emcc_args += ['-std=c++20', '--bind', '--post-js=post.js', '-fexceptions'] self.do_runf('embind/test_val_coro.cpp', 'rejected with: bang from JS promise!\n') - def test_embind_val_coro_caught_exception(self): - self.set_setting('EXCEPTION_STACK_TRACES') - create_file('post.js', r'''Module.onRuntimeInitialized = () => { - Module.caughtException().then(console.log); - }''') - self.emcc_args += ['-std=c++20', '--bind', '--post-js=post.js', '-fexceptions'] - self.do_runf('embind/test_val_coro.cpp', '42\n') - def test_embind_dynamic_initialization(self): self.emcc_args += ['-lembind'] self.do_run_in_out_file_test('embind/test_dynamic_initialization.cpp') From ede595544931fafdb237868a22f028f653a7b802 Mon Sep 17 00:00:00 2001 From: Davit Samvelyan Date: Mon, 24 Feb 2025 22:57:47 +0100 Subject: [PATCH 5/8] Turn embind into MTLibrary Newly added _emval_coro_reject() function calls JS Promise reject functor at the end of call hierarchy. Since the call crosses C++ JS barrier, the Signature::get_method_caller() function is called, which has thread_local variable, adding thread local storage, and therefore the requirement for the embind to be compiled with same threadness as the main program. --- tools/system_libs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/system_libs.py b/tools/system_libs.py index 1a2c90c3d05aa..842099e48a388 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -1925,7 +1925,7 @@ class libwebgpu_cpp(MTLibrary): src_files = ['webgpu_cpp.cpp'] -class libembind(Library): +class libembind(MTLibrary): name = 'libembind' never_force = True From 41eff7b85f340f29ef38ea732c7f14fd88910c68 Mon Sep 17 00:00:00 2001 From: Davit Samvelyan Date: Thu, 6 Mar 2025 16:03:37 +0100 Subject: [PATCH 6/8] Rename exception to val converting function. --- src/lib/libemval.js | 4 ++-- src/lib/libsigs.js | 2 +- system/include/emscripten/val.h | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lib/libemval.js b/src/lib/libemval.js index aa135b5051164..257294f311ee6 100644 --- a/src/lib/libemval.js +++ b/src/lib/libemval.js @@ -482,8 +482,8 @@ var LibraryEmVal = { })); }, - _emval_coro_exception_to_error__deps: ['$Emval', '__cxa_rethrow'], - _emval_coro_exception_to_error: () => { + _emval_from_current_cxa_exception__deps: ['$Emval', '__cxa_rethrow'], + _emval_from_current_cxa_exception: () => { try { // Use __cxa_rethrow which already has mechanism for generating // user-friendly error message and stacktrace from C++ exception diff --git a/src/lib/libsigs.js b/src/lib/libsigs.js index 33b4bf642b3e3..449caa86f6516 100644 --- a/src/lib/libsigs.js +++ b/src/lib/libsigs.js @@ -339,12 +339,12 @@ sigs = { _emval_await__sig: 'pp', _emval_call__sig: 'dpppp', _emval_call_method__sig: 'dppppp', - _emval_coro_exception_to_error__sig: 'p', _emval_coro_make_promise__sig: 'ppp', _emval_coro_suspend__sig: 'vpp', _emval_decref__sig: 'vp', _emval_delete__sig: 'ipp', _emval_equals__sig: 'ipp', + _emval_from_current_cxa_exception__sig: 'p', _emval_get_global__sig: 'pp', _emval_get_method_caller__sig: 'pipi', _emval_get_module_property__sig: 'pp', diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h index b080cb0057983..1b4804f4e0b01 100644 --- a/system/include/emscripten/val.h +++ b/system/include/emscripten/val.h @@ -118,7 +118,7 @@ EM_VAL _emval_iter_next(EM_VAL iterator); #if __cplusplus >= 202002L void _emval_coro_suspend(EM_VAL promise, void* coro_ptr); -EM_VAL _emval_coro_exception_to_error(); +EM_VAL _emval_from_current_cxa_exception(); EM_VAL _emval_coro_make_promise(EM_VAL *resolve, EM_VAL *reject); #endif @@ -793,7 +793,7 @@ class val::promise_type { } catch (const val& error) { reject(error); } catch (...) { - val error = val(internal::_emval_coro_exception_to_error()); + val error = val(internal::_emval_from_current_cxa_exception()); reject(error); } } From f3758df71bac31bfcf5487abdc43a1b003176462 Mon Sep 17 00:00:00 2001 From: Davit Samvelyan Date: Wed, 12 Mar 2025 17:42:51 +0100 Subject: [PATCH 7/8] Change JS promise error handling to then(). --- src/lib/libemval.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/libemval.js b/src/lib/libemval.js index 257294f311ee6..4404f9311ba4d 100644 --- a/src/lib/libemval.js +++ b/src/lib/libemval.js @@ -470,8 +470,8 @@ var LibraryEmVal = { _emval_coro_suspend__deps: ['$Emval', '_emval_coro_resume', '_emval_coro_reject'], _emval_coro_suspend: (promiseHandle, awaiterPtr) => { Emval.toValue(promiseHandle) - .then(result => __emval_coro_resume(awaiterPtr, Emval.toHandle(result))) - .catch(error => __emval_coro_reject(awaiterPtr, Emval.toHandle(error))); + .then(result => __emval_coro_resume(awaiterPtr, Emval.toHandle(result)), + error => __emval_coro_reject(awaiterPtr, Emval.toHandle(error))); }, _emval_coro_make_promise__deps: ['$Emval'], From 45010b576526da3ffc4fd5697331edde722df7af Mon Sep 17 00:00:00 2001 From: Davit Samvelyan Date: Thu, 8 May 2025 11:07:10 +0200 Subject: [PATCH 8/8] Fix coding style issue. --- src/lib/libemval.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/libemval.js b/src/lib/libemval.js index 07d1bd6a1f9a4..48eb6334601ad 100644 --- a/src/lib/libemval.js +++ b/src/lib/libemval.js @@ -459,8 +459,8 @@ var LibraryEmVal = { _emval_coro_suspend__deps: ['$Emval', '_emval_coro_resume', '_emval_coro_reject'], _emval_coro_suspend: (promiseHandle, awaiterPtr) => { Emval.toValue(promiseHandle) - .then(result => __emval_coro_resume(awaiterPtr, Emval.toHandle(result)), - error => __emval_coro_reject(awaiterPtr, Emval.toHandle(error))); + .then((result) => __emval_coro_resume(awaiterPtr, Emval.toHandle(result)), + (error) => __emval_coro_reject(awaiterPtr, Emval.toHandle(error))); }, _emval_coro_make_promise__deps: ['$Emval'],