Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement emscripten_promise_race in promise.h #19154

Merged
merged 2 commits into from
Apr 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/library_promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,21 @@ mergeInto(LibraryManager.library, {
});
#if RUNTIME_DEBUG
dbg('create: ' + id);
#endif
return id;
},

emscripten_promise_race__deps: ['$promiseMap', '$idsToPromises'],
emscripten_promise_race: function(idBuf, size) {
var promises = idsToPromises(idBuf, size);
#if RUNTIME_DEBUG
dbg('emscripten_promise_race: ' + promises);
#endif
var id = promiseMap.allocate({
promise: Promise.race(promises)
});
#if RUNTIME_DEBUG
dbg('create: ' + id);
#endif
return id;
}
Expand Down
1 change: 1 addition & 0 deletions src/library_sigs.js
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,7 @@ sigs = {
emscripten_promise_any__sig: 'pppp',
emscripten_promise_create__sig: 'p',
emscripten_promise_destroy__sig: 'vp',
emscripten_promise_race__sig: 'ppp',
emscripten_promise_resolve__sig: 'vpip',
emscripten_promise_then__sig: 'ppppp',
emscripten_random__sig: 'f',
Expand Down
8 changes: 8 additions & 0 deletions system/include/emscripten/promise.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,14 @@ __attribute__((warn_unused_result)) em_promise_t emscripten_promise_all_settled(
__attribute__((warn_unused_result)) em_promise_t emscripten_promise_any(
em_promise_t* promises, void** errors, size_t num_promises);

// Call Promise.race to create and return a new promise that settles once any of
// the `num_promises` input promises passed in `promises` has been settled. If
// the first input promise to settle is fulfilled, the resulting promise is
// fulfilled with the same value. Otherwise, if the first input promise to
// settle is rejected, the resulting promise is rejected with the same reason.
__attribute__((warn_unused_result)) em_promise_t
emscripten_promise_race(em_promise_t* promises, size_t num_promises);

#ifdef __cplusplus
}
#endif
41 changes: 40 additions & 1 deletion test/core/test_promise.c
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,42 @@ static em_promise_result_t test_any(void** result, void* data, void* value) {
return EM_PROMISE_MATCH_RELEASE;
}

static em_promise_result_t test_race(void** result, void* data, void* value) {
emscripten_console_log("test_race");
assert(data == (void*)7);

em_promise_t in[2] = {emscripten_promise_create(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that it matters but I think you can drop the "2" here and below and have it inferred. Might make the code less readable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's true. I kind of like having the explicit visual marker where I can check that the declared size matches the size I pass to the API function, but I don't think it matters too much.

emscripten_promise_create()};
em_promise_t fulfill = emscripten_promise_race(in, 2);
em_promise_t fulfill_checked =
emscripten_promise_then(fulfill, expect_success, fail, NULL);
emscripten_promise_destroy(fulfill);
emscripten_promise_resolve(in[1], EM_PROMISE_FULFILL, (void*)42);
emscripten_promise_resolve(in[0], EM_PROMISE_REJECT, NULL);
emscripten_promise_destroy(in[0]);
emscripten_promise_destroy(in[1]);

in[0] = emscripten_promise_create();
in[1] = emscripten_promise_create();
em_promise_t reject = emscripten_promise_race(in, 2);
em_promise_t reject_checked =
emscripten_promise_then(reject, fail, expect_error, NULL);
emscripten_promise_destroy(reject);
emscripten_promise_resolve(in[0], EM_PROMISE_REJECT, (void*)42);
emscripten_promise_resolve(in[1], EM_PROMISE_FULFILL, NULL);
emscripten_promise_destroy(in[0]);
emscripten_promise_destroy(in[1]);

em_promise_t to_finish[2] = {fulfill_checked, reject_checked};
em_promise_t finish_test_race = emscripten_promise_all(to_finish, NULL, 0);

emscripten_promise_destroy(fulfill_checked);
emscripten_promise_destroy(reject_checked);

*result = finish_test_race;
return EM_PROMISE_MATCH_RELEASE;
}

static em_promise_result_t finish(void** result, void* data, void* value) {
emscripten_console_logf("finish");

Expand Down Expand Up @@ -509,8 +545,10 @@ int main() {
em_promise_t test5 =
emscripten_promise_then(test4, test_all_settled, fail, (void*)5);
em_promise_t test6 = emscripten_promise_then(test5, test_any, fail, (void*)6);
em_promise_t test7 =
emscripten_promise_then(test6, test_race, fail, (void*)7);
em_promise_t assert_stack =
emscripten_promise_then(test6, check_stack, fail, NULL);
emscripten_promise_then(test7, check_stack, fail, NULL);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this kind of test that has many phases/sub-tests, would make sense written in gtest? I added one test recently that uses gtest, but perhaps it would be worth looking into using it more? Once downside is that its maybe a little heavy weight and relies heavily on C++ (so is not so good for testing low level C libraries perhaps?)>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gtest is great, but another problem for these tests in particular is that I don't think gtest would play well with the asynchrony happening here. The actual test checks for all of these tests aren't being run until well after main returns.

em_promise_t end = emscripten_promise_then(assert_stack, finish, fail, NULL);

emscripten_promise_resolve(start, EM_PROMISE_FULFILL, NULL);
Expand All @@ -523,6 +561,7 @@ int main() {
emscripten_promise_destroy(test4);
emscripten_promise_destroy(test5);
emscripten_promise_destroy(test6);
emscripten_promise_destroy(test7);
emscripten_promise_destroy(assert_stack);
emscripten_promise_destroy(end);

Expand Down
3 changes: 3 additions & 0 deletions test/core/test_promise.out
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,7 @@ promise_any reasons:
42
43
44
test_race
expected success: 42
expected error: 42
finish