Skip to content

[test] Fix flakiness in test_pthread_wait32/64_notify #20412

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 11, 2023
Merged

[test] Fix flakiness in test_pthread_wait32/64_notify #20412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions test/test_browser.py
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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.
Copy link
Member

Choose a reason for hiding this comment

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

This PR seems like an improvement, but as a side note, this part of the test ("burn one second") looks fishy...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree that is pretty nasty.. but its also pre-existing. I looked into trying to fix that but it could be more involved so leaving it for now.

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
View file
Open in desktop
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
View file
Open in desktop
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