Skip to content

Commit 3f7dff6

Browse files
committed
Improve emscripten_futex_wait stub. NFC
The test for wasm workers with `emscripten_futex_wait` was added in back in #21618 but AFAICT it was only even working when pthread support was also enabled. However because the stub was simply returning 0 in all cases it was enough to make it seems as if the API was working when it wasn't. Indeed the `test/webaudio/audioworklet_worker.c` test seems to have been written under the assumption that this API was available for use in wasm workers. It seems better to return ENOTSUP unless the API is actually available.
1 parent 8119447 commit 3f7dff6

5 files changed

Lines changed: 47 additions & 20 deletions

File tree

system/lib/pthread/library_pthread_stub.c

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,16 @@ bool emscripten_has_threading_support() { return false; }
2121

2222
int emscripten_num_logical_cores() { return 1; }
2323

24-
int emscripten_futex_wait(
25-
volatile void /*uint32_t*/* addr, uint32_t val, double maxWaitMilliseconds) {
26-
// nop
27-
return 0; // success
24+
int emscripten_futex_wait(volatile void /*uint32_t*/* addr,
25+
uint32_t val,
26+
double maxWaitMilliseconds) {
27+
if (!addr) {
28+
return -EINVAL;
29+
}
30+
if (*(uint32_t*)addr != val) {
31+
return -EWOULDBLOCK;
32+
}
33+
return -ENOTSUP;
2834
}
2935

3036
int emscripten_futex_wake(volatile void /*uint32_t*/* addr, int count) {

test/codesize/test_codesize_hello_dylink_all.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"a.out.js": 244691,
3-
"a.out.nodebug.wasm": 577678,
4-
"total": 822369,
3+
"a.out.nodebug.wasm": 577699,
4+
"total": 822390,
55
"sent": [
66
"IMG_Init",
77
"IMG_Load",

test/test_browser.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5039,9 +5039,11 @@ def test_wasm_worker_hello_wasm2js(self):
50395039
def test_wasm_worker_hello_embedded(self):
50405040
self.btest_exit('wasm_worker/hello_wasm_worker.c', cflags=['-sWASM_WORKERS=2'])
50415041

5042-
# Tests that it is possible to call emscripten_futex_wait() in Wasm Workers.
5042+
# Tests that it is possible to call emscripten_futex_wait() in Wasm Workers when pthreads
5043+
# are also enabled.
50435044
@parameterized({
5044-
'': ([],),
5045+
# Without pthreads we expect the stub version of the futex API
5046+
'': (['-DEXPECT_STUB'],),
50455047
'pthread': (['-pthread'],),
50465048
})
50475049
def test_wasm_worker_futex_wait(self, args):

test/wasm_worker/wasm_worker_futex_wait.c

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// Test that emscripten_futex_wait() works in a Wasm Worker.
22

3+
#include <assert.h>
4+
#include <errno.h>
35
#include <emscripten.h>
46
#include <emscripten/console.h>
57
#include <emscripten/threading.h>
@@ -12,21 +14,37 @@ _Atomic uint32_t futex_value = 0;
1214

1315
void wake_worker_after_delay(void *user_data) {
1416
futex_value = 1;
15-
emscripten_futex_wake(&futex_value, INT_MAX);
17+
emscripten_futex_wake(&futex_value, INT_MAX);
1618
}
1719

1820
void wake_worker() {
19-
printf("Waking worker thread from futex wait.\n");
21+
printf("Waking worker thread from futex wait...\n");
2022
emscripten_set_timeout(wake_worker_after_delay, 500, 0);
2123
}
2224

2325
void worker_main() {
24-
printf("Worker sleeping for futex wait.\n");
26+
printf("Worker sleeping on futex...\n");
27+
double start = emscripten_performance_now();
28+
int rc = emscripten_futex_wait(&futex_value, 0, 100);
29+
double end = emscripten_performance_now();
30+
printf("emscripten_futex_wait returned: %d after %.2fms\n", rc, end - start);
31+
#if EXPECT_STUB
32+
// The stub implemenation returns -ENOTSUP
33+
assert(rc == -ENOTSUP);
34+
#else
35+
assert(rc == -ETIMEDOUT);
36+
assert((end - start) >= 100);
37+
38+
printf("Worker sleeping on futex with wakeup...\n");
2539
emscripten_wasm_worker_post_function_v(0, wake_worker);
26-
int rc = emscripten_futex_wait(&futex_value, 0, INFINITY);
27-
printf("emscripten_futex_wait returned with code %d.\n", rc);
40+
rc = emscripten_futex_wait(&futex_value, 0, INFINITY);
41+
printf("emscripten_futex_wait returned: %d\n", rc);
42+
assert(rc == 0);
43+
assert(futex_value == 1);
44+
#endif
45+
2846
#ifdef REPORT_RESULT
29-
REPORT_RESULT(rc);
47+
REPORT_RESULT(0);
3048
#endif
3149
}
3250

test/webaudio/audioworklet_worker.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22
#include <emscripten/wasm_worker.h>
33
#include <emscripten/threading.h>
44
#include <assert.h>
5+
#include <stdbool.h>
56

67
// Tests that
78
// - audioworklets and workers can be used at the same time.
89
// - an audioworklet can emscripten_futex_wake() a waiting worker.
910
// - global values can be shared between audioworklets and workers.
1011

11-
int workletToWorkerFutexLocation = 0;
12-
int workletToWorkerFlag = 0;
12+
_Atomic bool workletToWorkerFlag = false;
1313
EMSCRIPTEN_WEBAUDIO_T context;
1414

1515
void do_exit() {
@@ -19,8 +19,10 @@ void do_exit() {
1919
}
2020

2121
void run_in_worker() {
22-
while (0 == emscripten_futex_wait(&workletToWorkerFutexLocation, 0, 30000)) {
23-
if (workletToWorkerFlag == 1) {
22+
// TODO: Convert to emscripten_futex_wait once it becomes available in Wasm
23+
// Workers.
24+
while (1) {
25+
if (workletToWorkerFlag == true) {
2426
emscripten_out("Test success");
2527
emscripten_wasm_worker_post_function_v(EMSCRIPTEN_WASM_WORKER_ID_PARENT, &do_exit);
2628
break;
@@ -31,8 +33,7 @@ void run_in_worker() {
3133
// This event will fire on the audio worklet thread.
3234
void MessageReceivedInAudioWorkletThread() {
3335
assert(emscripten_current_thread_is_audio_worklet());
34-
workletToWorkerFlag = 1;
35-
emscripten_futex_wake(&workletToWorkerFutexLocation, 1);
36+
workletToWorkerFlag = true;
3637
}
3738

3839
void WebAudioWorkletThreadInitialized(EMSCRIPTEN_WEBAUDIO_T audioContext, bool success, void *userData) {

0 commit comments

Comments
 (0)