Skip to content

Commit

Permalink
[test] Fix flakiness in test_pthread_wait32/64_notify (emscripten-cor…
Browse files Browse the repository at this point in the history
…e#20412)

Avoid racing between the main thread and the worker by only triggering
the main thread to notify once the workers done printing all its
messages.
  • Loading branch information
sbc100 committed Oct 11, 2023
1 parent dcc9d4b commit 4b33521
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 10 deletions.
4 changes: 2 additions & 2 deletions test/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -5296,12 +5296,12 @@ def test_wasm_worker_hardware_concurrency_is_lock_free(self):
# Tests emscripten_atomic_wait_u32() and emscripten_atomic_notify() functions.
@also_with_minimal_runtime
def test_wasm_worker_wait32_notify(self):
self.btest('wasm_worker/wait32_notify.c', expected='2', args=['-sWASM_WORKERS'])
self.btest('wasm_worker/wait32_notify.c', expected='3', args=['-sWASM_WORKERS'])

# Tests emscripten_atomic_wait_u64() and emscripten_atomic_notify() functions.
@also_with_minimal_runtime
def test_wasm_worker_wait64_notify(self):
self.btest('wasm_worker/wait64_notify.c', expected='2', args=['-sWASM_WORKERS'])
self.btest('wasm_worker/wait64_notify.c', expected='3', args=['-sWASM_WORKERS'])

# Tests emscripten_atomic_wait_async() function.
@also_with_minimal_runtime
Expand Down
2 changes: 2 additions & 0 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -2923,11 +2923,13 @@ def test_pthread_run_script(self):

@node_pthreads
def test_pthread_wait32_notify(self):
self.set_setting('EXIT_RUNTIME')
self.do_run_in_out_file_test(test_file('wasm_worker/wait32_notify.c'))

@node_pthreads
@no_wasm2js('https://github.com/WebAssembly/binaryen/issues/5991')
def test_pthread_wait64_notify(self):
self.set_setting('EXIT_RUNTIME')
self.do_run_in_out_file_test(test_file('wasm_worker/wait64_notify.c'))

def test_tcgetattr(self):
Expand Down
9 changes: 4 additions & 5 deletions test/wasm_worker/wait32_notify.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ void run_test() {
assert(ret == ATOMICS_WAIT_TIMED_OUT);

emscripten_out("Waiting for infinitely long should return 'ok'");
ret = emscripten_atomic_wait_u32((int32_t*)&addr, 1, /*timeout=*/-1);
emscripten_atomic_store_u32((void*)&addr, 3);
ret = emscripten_atomic_wait_u32((int32_t*)&addr, 3, /*timeout=*/-1);
assert(ret == ATOMICS_WAIT_OK);

emscripten_out("Test finished");
Expand All @@ -43,8 +44,6 @@ void run_test() {
// This test run in both wasm workers and pthreads mode
#ifdef __EMSCRIPTEN_WASM_WORKERS__

char stack[1024];

void worker_main() {
run_test();

Expand All @@ -66,15 +65,14 @@ void* thread_main(void* arg) {


EM_BOOL main_loop(double time, void *userData) {
if (addr == 1) {
if (addr == 3) {
// Burn one second to make sure worker finishes its test.
emscripten_out("main: seen worker running");
double t0 = emscripten_performance_now();
while(emscripten_performance_now() < t0 + 1000);

// Wake the waiter
emscripten_out("main: waking worker");
addr = 2;
emscripten_atomic_notify((int32_t*)&addr, 1);

#ifndef __EMSCRIPTEN_WASM_WORKERS__
Expand All @@ -90,6 +88,7 @@ int main() {
emscripten_out("main: creating worker");

#ifdef __EMSCRIPTEN_WASM_WORKERS__
static char stack[1024];
emscripten_wasm_worker_t worker = emscripten_create_wasm_worker(stack, sizeof(stack));
emscripten_wasm_worker_post_function_v(worker, worker_main);
#else
Expand Down
6 changes: 3 additions & 3 deletions test/wasm_worker/wait64_notify.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ void run_test() {
assert(ret == ATOMICS_WAIT_TIMED_OUT);

emscripten_out("Waiting for infinitely long should return 'ok'");
ret = emscripten_atomic_wait_u64((int64_t*)&addr, 0x100000000ull, /*timeout=*/-1);
emscripten_atomic_store_u64((void*)&addr, 0x300000000ull);
ret = emscripten_atomic_wait_u64((int64_t*)&addr, 0x300000000ull, /*timeout=*/-1);
assert(ret == ATOMICS_WAIT_OK);

emscripten_out("Test finished");
Expand Down Expand Up @@ -65,15 +66,14 @@ void* thread_main(void* arg) {
#endif

EM_BOOL main_loop(double time, void *userData) {
if (addr == 0x100000000ull) {
if (addr == 0x300000000ull) {
// Burn one second to make sure worker finishes its test.
emscripten_out("main: seen worker running");
double t0 = emscripten_performance_now();
while(emscripten_performance_now() < t0 + 1000);

// Wake the waiter
emscripten_out("main: waking worker");
addr = 0x200000000ull;
emscripten_atomic_notify((int32_t*)&addr, 1);

return EM_FALSE;
Expand Down
1 change: 1 addition & 0 deletions test/wasm_worker/wait64_notify.out
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ Waiting for >0 nanoseconds should return 'timed-out'
Waiting for infinitely long should return 'ok'
main: seen worker running
main: waking worker
Test finished

0 comments on commit 4b33521

Please sign in to comment.