Skip to content

Commit 5317e84

Browse files
committed
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.
1 parent a9651ff commit 5317e84

File tree

5 files changed

+52
-26
lines changed

5 files changed

+52
-26
lines changed

src/lib/libemval.js

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -467,33 +467,33 @@ var LibraryEmVal = {
467467
return result.done ? 0 : Emval.toHandle(result.value);
468468
},
469469

470-
_emval_coro_suspend__deps: ['$Emval', '_emval_coro_resume'],
471-
_emval_coro_suspend: async (promiseHandle, awaiterPtr) => {
472-
var result = await Emval.toValue(promiseHandle);
473-
__emval_coro_resume(awaiterPtr, Emval.toHandle(result));
470+
_emval_coro_suspend__deps: ['$Emval', '_emval_coro_resume', '_emval_coro_reject'],
471+
_emval_coro_suspend: (promiseHandle, awaiterPtr) => {
472+
Emval.toValue(promiseHandle)
473+
.then(result => __emval_coro_resume(awaiterPtr, Emval.toHandle(result)))
474+
.catch(error => __emval_coro_reject(awaiterPtr, Emval.toHandle(error)));
474475
},
475476

476-
_emval_coro_make_promise__deps: ['$Emval', '__cxa_rethrow'],
477+
_emval_coro_make_promise__deps: ['$Emval'],
477478
_emval_coro_make_promise: (resolveHandlePtr, rejectHandlePtr) => {
478479
return Emval.toHandle(new Promise((resolve, reject) => {
479-
const rejectWithCurrentException = () => {
480-
try {
481-
// Use __cxa_rethrow which already has mechanism for generating
482-
// user-friendly error message and stacktrace from C++ exception
483-
// if EXCEPTION_STACK_TRACES is enabled and numeric exception
484-
// with metadata optimised out otherwise.
485-
___cxa_rethrow();
486-
} catch (e) {
487-
// But catch it so that it rejects the promise instead of throwing
488-
// in an unpredictable place during async execution.
489-
reject(e);
490-
}
491-
};
492-
493480
{{{ makeSetValue('resolveHandlePtr', '0', 'Emval.toHandle(resolve)', '*') }}};
494-
{{{ makeSetValue('rejectHandlePtr', '0', 'Emval.toHandle(rejectWithCurrentException)', '*') }}};
481+
{{{ makeSetValue('rejectHandlePtr', '0', 'Emval.toHandle(reject)', '*') }}};
495482
}));
496483
},
484+
485+
_emval_coro_exception_to_error__deps: ['$Emval', '__cxa_rethrow'],
486+
_emval_coro_exception_to_error: () => {
487+
try {
488+
// Use __cxa_rethrow which already has mechanism for generating
489+
// user-friendly error message and stacktrace from C++ exception
490+
// if EXCEPTION_STACK_TRACES is enabled and numeric exception
491+
// with metadata optimised out otherwise.
492+
___cxa_rethrow();
493+
} catch (e) {
494+
return Emval.toHandle(e);
495+
}
496+
},
497497
};
498498

499499
addToLibrary(LibraryEmVal);

src/lib/libsigs.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,7 @@ sigs = {
338338
_emval_await__sig: 'pp',
339339
_emval_call__sig: 'dpppp',
340340
_emval_call_method__sig: 'dppppp',
341+
_emval_coro_exception_to_error__sig: 'p',
341342
_emval_coro_make_promise__sig: 'ppp',
342343
_emval_coro_suspend__sig: 'vpp',
343344
_emval_decref__sig: 'vp',

system/include/emscripten/val.h

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
#include <variant>
2525
#endif
2626

27-
2827
namespace emscripten {
2928

3029
class val;
@@ -118,6 +117,7 @@ EM_VAL _emval_iter_next(EM_VAL iterator);
118117

119118
#if __cplusplus >= 202002L
120119
void _emval_coro_suspend(EM_VAL promise, void* coro_ptr);
120+
EM_VAL _emval_coro_exception_to_error();
121121
EM_VAL _emval_coro_make_promise(EM_VAL *resolve, EM_VAL *reject);
122122
#endif
123123

@@ -712,11 +712,13 @@ class val::awaiter {
712712
// - initially created with promise
713713
// - waiting with a given coroutine handle
714714
// - completed with a result
715-
std::variant<val, std::coroutine_handle<val::promise_type>, val> state;
715+
// - rejected with an error
716+
std::variant<val, std::coroutine_handle<val::promise_type>, val, val> state;
716717

717718
constexpr static std::size_t STATE_PROMISE = 0;
718719
constexpr static std::size_t STATE_CORO = 1;
719720
constexpr static std::size_t STATE_RESULT = 2;
721+
constexpr static std::size_t STATE_ERROR = 3;
720722

721723
public:
722724
awaiter(const val& promise)
@@ -743,9 +745,20 @@ class val::awaiter {
743745
coro.resume();
744746
}
745747

748+
void reject_with(val&& error) {
749+
auto coro = std::move(std::get<STATE_CORO>(state));
750+
state.emplace<STATE_ERROR>(std::move(error));
751+
coro.resume();
752+
}
753+
746754
// `await_resume` finalizes the awaiter and should return the result
747755
// of the `co_await ...` expression - in our case, the stored value.
748-
val await_resume() { return std::move(std::get<STATE_RESULT>(state)); }
756+
val await_resume() {
757+
if (state.index() == STATE_ERROR) {
758+
throw std::get<STATE_ERROR>(state);
759+
}
760+
return std::move(std::get<STATE_RESULT>(state));
761+
}
749762
};
750763

751764
inline val::awaiter val::operator co_await() const {
@@ -756,7 +769,7 @@ inline val::awaiter val::operator co_await() const {
756769
// that compiler uses to drive the coroutine itself
757770
// (`T::promise_type` is used for any coroutine with declared return type `T`).
758771
class val::promise_type {
759-
val promise, resolve, reject_with_current_exception;
772+
val promise, resolve, reject;
760773

761774
public:
762775
// Create a `new Promise` and store it alongside the `resolve` and `reject`
@@ -766,7 +779,7 @@ class val::promise_type {
766779
EM_VAL reject_handle;
767780
promise = val(internal::_emval_coro_make_promise(&resolve_handle, &reject_handle));
768781
resolve = val(resolve_handle);
769-
reject_with_current_exception = val(reject_handle);
782+
reject = val(reject_handle);
770783
}
771784

772785
// Return the stored promise as the actual return value of the coroutine.
@@ -779,7 +792,14 @@ class val::promise_type {
779792
// On an unhandled exception, reject the stored promise instead of throwing
780793
// it asynchronously where it can't be handled.
781794
void unhandled_exception() {
782-
reject_with_current_exception();
795+
try {
796+
std::rethrow_exception(std::current_exception());
797+
} catch (const val& error) {
798+
reject(error);
799+
} catch (...) {
800+
val error = val(internal::_emval_coro_exception_to_error());
801+
reject(error);
802+
}
783803
}
784804

785805
// Resolve the stored promise on `co_return value`.

system/lib/embind/bind.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ void _emval_coro_resume(val::awaiter* awaiter, EM_VAL result) {
6767
awaiter->resume_with(val::take_ownership(result));
6868
}
6969

70+
void _emval_coro_reject(val::awaiter* awaiter, EM_VAL error) {
71+
awaiter->reject_with(val::take_ownership(error));
72+
}
73+
7074
}
7175

7276
namespace {

tools/emscripten.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,6 +1093,7 @@ def create_pointer_conversion_wrappers(metadata):
10931093
'emscripten_proxy_finish': '_p',
10941094
'emscripten_proxy_execute_queue': '_p',
10951095
'_emval_coro_resume': '_pp',
1096+
'_emval_coro_reject': '_pp',
10961097
'emscripten_main_runtime_thread_id': 'p',
10971098
'_emscripten_set_offscreencanvas_size_on_thread': '_pp__',
10981099
'fileno': '_p',

0 commit comments

Comments
 (0)