Skip to content

Commit a14cbf7

Browse files
committed
Fix timeout in emscripten_semaphore_wait_acquire
This fixes the TODO in this function which meant that the timeout was not correctly honored in the case of multiple wakeups. The code here is basically the same as the existing code in `emscripten_lock_wait_acquire` just above.
1 parent cb060ca commit a14cbf7

File tree

4 files changed

+89
-4
lines changed

4 files changed

+89
-4
lines changed

system/lib/pthread/emscripten_thread_primitives.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,15 @@ int emscripten_semaphore_try_acquire(emscripten_semaphore_t *sem, int num) {
8787

8888
int emscripten_semaphore_wait_acquire(emscripten_semaphore_t *sem, int num, int64_t maxWaitNanoseconds) {
8989
int val = emscripten_atomic_load_u32((void*)sem);
90+
int64_t waitEnd = (int64_t)(emscripten_performance_now() * 1e6) + maxWaitNanoseconds;
9091
for (;;) {
91-
while (val < num) {
92-
// TODO: Shave off maxWaitNanoseconds
93-
ATOMICS_WAIT_RESULT_T waitResult = emscripten_atomic_wait_u32((void*)sem, val, maxWaitNanoseconds);
94-
if (waitResult == ATOMICS_WAIT_TIMED_OUT) return -1;
92+
while (val < num && maxWaitNanoseconds > 0) {
93+
emscripten_atomic_wait_u32((void*)sem, val, maxWaitNanoseconds);
9594
val = emscripten_atomic_load_u32((void*)sem);
95+
maxWaitNanoseconds = waitEnd - (int64_t)(emscripten_performance_now() * 1e6);
9696
}
97+
// If we exited the loop and still don't have enough, it means we timed out.
98+
if (val < num) return -1;
9799
int ret = (int)emscripten_atomic_cas_u32((void*)sem, val, val - num);
98100
if (ret == val) return val - num;
99101
val = ret;

test/test_browser.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5219,6 +5219,11 @@ def test_wasm_worker_no_proxied_js_functions(self):
52195219
def test_wasm_worker_semaphore_waitinf_acquire(self):
52205220
self.btest('wasm_worker/semaphore_waitinf_acquire.c', expected='0', cflags=['-sWASM_WORKERS'])
52215221

5222+
# Tests emscripten_semaphore_wait_acquire()
5223+
@also_with_minimal_runtime
5224+
def test_wasm_worker_semaphore_wait_acquire(self):
5225+
self.btest('wasm_worker/semaphore_wait_acquire.c', expected='0', cflags=['-sWASM_WORKERS'])
5226+
52225227
# Tests emscripten_semaphore_try_acquire() on the main thread
52235228
@also_with_minimal_runtime
52245229
def test_wasm_worker_semaphore_try_acquire(self):

test/test_core.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,6 +2789,11 @@ def test_emscripten_lock_busyspin_waitinf(self):
27892789
def test_emscripten_semaphore_waitinf_acquire(self):
27902790
self.do_runf('wasm_worker/semaphore_waitinf_acquire.c', 'done\n', cflags=['-pthread', '-sPTHREAD_POOL_SIZE=7'])
27912791

2792+
@requires_pthreads
2793+
@also_with_wasm_workers
2794+
def test_emscripten_semaphore_wait_acquire(self):
2795+
self.do_runf('wasm_worker/semaphore_wait_acquire.c', 'done\n', cflags=['-pthread'])
2796+
27922797
@requires_pthreads
27932798
@also_with_wasm_workers
27942799
def test_emscripten_semaphore_try_acquire(self):
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
#include <emscripten.h>
2+
#include <emscripten/threading.h>
3+
#include <emscripten/wasm_worker.h>
4+
5+
#include <assert.h>
6+
#include <stdio.h>
7+
#include <stdlib.h>
8+
9+
#ifdef __EMSCRIPTEN_PTHREADS__
10+
#include <pthread.h>
11+
#endif
12+
13+
// Tests emscripten_semaphore_wait_acquire() and
14+
// emscripten_semaphore_try_acquire() in Worker.
15+
16+
emscripten_semaphore_t sem = EMSCRIPTEN_SEMAPHORE_T_STATIC_INITIALIZER(0);
17+
18+
void worker_main() {
19+
// 1. Immediate timeout (maxWaitNanoseconds = 0) when unavailable.
20+
double t0 = emscripten_performance_now();
21+
int ret = emscripten_semaphore_wait_acquire(&sem, 1, 0);
22+
double t1 = emscripten_performance_now();
23+
assert(ret == -1);
24+
assert(t1 - t0 < 50); // Should be near-instant
25+
assert(sem == 0); // Should not have been decremented
26+
27+
// 2. Timed wait that times out.
28+
t0 = emscripten_performance_now();
29+
ret = emscripten_semaphore_wait_acquire(&sem, 1, 100 * 1000000ull); // 100ms
30+
t1 = emscripten_performance_now();
31+
assert(ret == -1);
32+
assert(t1 - t0 >= 100);
33+
assert(sem == 0); // Should not have been decremented
34+
35+
// 3. Immediate acquisition when available.
36+
emscripten_semaphore_release(&sem, 2);
37+
assert(sem == 2);
38+
ret = emscripten_semaphore_wait_acquire(&sem, 1, 100 * 1000000ull);
39+
assert(ret == 1); // returns old value minus 1
40+
// If sem was 2, and we acquire 1, it returns 2 - 1 = 1.
41+
assert(sem == 1);
42+
43+
// 4. Try acquire
44+
ret = emscripten_semaphore_try_acquire(&sem, 1);
45+
assert(ret == 0);
46+
assert(sem == 0);
47+
48+
#ifdef REPORT_RESULT
49+
REPORT_RESULT(0);
50+
#endif
51+
}
52+
53+
#ifdef __EMSCRIPTEN_PTHREADS__
54+
void* thread_main(void* arg) {
55+
worker_main();
56+
return NULL;
57+
}
58+
#else
59+
char stack[1024];
60+
#endif
61+
62+
int main() {
63+
#ifdef __EMSCRIPTEN_PTHREADS__
64+
pthread_t t;
65+
pthread_create(&t, NULL, thread_main, NULL);
66+
pthread_join(t, NULL);
67+
printf("done\n");
68+
#else
69+
emscripten_wasm_worker_t worker = emscripten_create_wasm_worker(stack, sizeof(stack));
70+
emscripten_wasm_worker_post_function_v(worker, worker_main);
71+
#endif
72+
return 0;
73+
}

0 commit comments

Comments
 (0)