From 59aac0a0ccef466d6ab031330e7b2595ac5f380b Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Tue, 23 Nov 2021 13:00:29 +0100 Subject: [PATCH 01/14] Simplify/unify wait in slices logic. NFC. --- system/lib/libc/musl/src/thread/__timedwait.c | 41 ++++++------ system/lib/libc/musl/src/thread/__wait.c | 21 +++--- .../musl/src/thread/pthread_barrier_wait.c | 9 ++- system/lib/pthread/library_pthread.c | 64 ++++++++----------- 4 files changed, 66 insertions(+), 69 deletions(-) diff --git a/system/lib/libc/musl/src/thread/__timedwait.c b/system/lib/libc/musl/src/thread/__timedwait.c index bdf18716e0ce3..15b51b4e8a0ef 100644 --- a/system/lib/libc/musl/src/thread/__timedwait.c +++ b/system/lib/libc/musl/src/thread/__timedwait.c @@ -5,7 +5,6 @@ #include #include #include -#include "pthread_impl.h" #else #include "futex.h" #endif @@ -33,10 +32,10 @@ static int __futex4_cp(volatile void *addr, int op, int val, const struct timesp if (r != -ENOSYS) return r; return __syscall_cp(SYS_futex, addr, op & ~FUTEX_PRIVATE, val, to); } -#endif static volatile int dummy = 0; weak_alias(dummy, __eintr_valid_flag); +#endif int __timedwait_cp(volatile int *addr, int val, clockid_t clk, const struct timespec *at, int priv) @@ -44,7 +43,9 @@ int __timedwait_cp(volatile int *addr, int val, int r; struct timespec to, *top=0; +#ifndef __EMSCRIPTEN__ if (priv) priv = FUTEX_PRIVATE; +#endif if (at) { if (at->tv_nsec >= 1000000000UL) return EINVAL; @@ -57,36 +58,34 @@ int __timedwait_cp(volatile int *addr, int val, if (to.tv_sec < 0) return ETIMEDOUT; top = &to; } - #ifdef __EMSCRIPTEN__ - double msecsToSleep = top ? (top->tv_sec * 1000 + top->tv_nsec / 1000000.0) : INFINITY; - int is_runtime_thread = emscripten_is_main_runtime_thread(); + pthread_t self = __pthread_self(); + double msecsToSleep = top ? (top->tv_sec * 1000.0 + top->tv_nsec / 1000000.0) : INFINITY; + const int is_runtime_thread = emscripten_is_main_runtime_thread(); + + // Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. + const double maxMsecsToSleep = is_runtime_thread ? 1 : 100; + // cp suffix in the function name means "cancellation point", so this wait can be cancelled - // by the users unless current threads cancelability is set to PTHREAD_CANCEL_DISABLE + // by the users unless current threads cancellability is set to PTHREAD_CANCEL_DISABLE // which may be either done by the user of __timedwait() function. - if (is_runtime_thread || - pthread_self()->canceldisable != PTHREAD_CANCEL_DISABLE || - pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) { + if (is_runtime_thread || self->canceldisable != PTHREAD_CANCEL_DISABLE || self->cancelasync) { double sleepUntilTime = emscripten_get_now() + msecsToSleep; do { - if (pthread_self()->cancel) { + if (self->cancel) { // Emscripten-specific return value: The wait was canceled by user calling // pthread_cancel() for this thread, and the caller needs to cooperatively // cancel execution. return ECANCELED; } + // Must wait in slices in case this thread is cancelled in between. + r = -emscripten_futex_wait((void*)addr, val, + msecsToSleep > maxMsecsToSleep ? maxMsecsToSleep : msecsToSleep); // Assist other threads by executing proxied operations that are effectively singlethreaded. if (is_runtime_thread) emscripten_main_thread_process_queued_calls(); - // Must wait in slices in case this thread is cancelled in between. - double waitMsecs = sleepUntilTime - emscripten_get_now(); - if (waitMsecs <= 0) { - r = ETIMEDOUT; - break; - } - if (waitMsecs > 100) waitMsecs = 100; // non-main threads can sleep in longer slices. - if (is_runtime_thread && waitMsecs > 1) waitMsecs = 1; // the runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. - r = -emscripten_futex_wait((void*)addr, val, waitMsecs); - } while(r == ETIMEDOUT); + + msecsToSleep = sleepUntilTime - emscripten_get_now(); + } while (r == ETIMEDOUT && msecsToSleep > 0); } else { // Can wait in one go. r = -emscripten_futex_wait((void*)addr, val, msecsToSleep); @@ -95,11 +94,13 @@ int __timedwait_cp(volatile int *addr, int val, r = -__futex4_cp(addr, FUTEX_WAIT|priv, val, top); #endif if (r != EINTR && r != ETIMEDOUT && r != ECANCELED) r = 0; +#ifndef __EMSCRIPTEN__ // XXX Emscripten revert musl commit a63c0104e496f7ba78b64be3cd299b41e8cd427f /* Mitigate bug in old kernels wrongly reporting EINTR for non- * interrupting (SA_RESTART) signal handlers. This is only practical * when NO interrupting signal handlers have been installed, and * works by sigaction tracking whether that's the case. */ if (r == EINTR && !__eintr_valid_flag) r = 0; +#endif return r; } diff --git a/system/lib/libc/musl/src/thread/__wait.c b/system/lib/libc/musl/src/thread/__wait.c index 712f6b9a49e8f..c5eca7935d627 100644 --- a/system/lib/libc/musl/src/thread/__wait.c +++ b/system/lib/libc/musl/src/thread/__wait.c @@ -8,29 +8,34 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv) { int spins=100; +#ifndef __EMSCRIPTEN__ if (priv) priv = FUTEX_PRIVATE; +#endif while (spins-- && (!waiters || !*waiters)) { if (*addr==val) a_spin(); else return; } if (waiters) a_inc(waiters); #ifdef __EMSCRIPTEN__ - int is_runtime_thread = emscripten_is_main_runtime_thread(); + pthread_t self = __pthread_self(); + const int is_runtime_thread = emscripten_is_main_runtime_thread(); + + // Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. + const double maxMsecsToSleep = is_runtime_thread ? 1 : 100; + while (*addr==val) { - if (is_runtime_thread || pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) { - // Must wait in slices in case this thread is cancelled in between. + if (is_runtime_thread || self->cancelasync) { int e; do { - if (pthread_self()->cancel) { + if (self->cancel) { if (waiters) a_dec(waiters); return; } + // Must wait in slices in case this thread is cancelled in between. + e = emscripten_futex_wait((void*)addr, val, maxMsecsToSleep); // Assist other threads by executing proxied operations that are effectively singlethreaded. if (is_runtime_thread) emscripten_main_thread_process_queued_calls(); - // Main thread waits in _very_ small slices so that it stays responsive to assist proxied - // pthread calls. - e = emscripten_futex_wait((void*)addr, val, is_runtime_thread ? 1 : 100); - } while(e == -ETIMEDOUT); + } while (e == -ETIMEDOUT); } else { // Can wait in one go. emscripten_futex_wait((void*)addr, val, INFINITY); diff --git a/system/lib/libc/musl/src/thread/pthread_barrier_wait.c b/system/lib/libc/musl/src/thread/pthread_barrier_wait.c index af2dde2eb247d..a254bbccdcd5d 100644 --- a/system/lib/libc/musl/src/thread/pthread_barrier_wait.c +++ b/system/lib/libc/musl/src/thread/pthread_barrier_wait.c @@ -88,17 +88,16 @@ int pthread_barrier_wait(pthread_barrier_t *b) a_spin(); a_inc(&inst->finished); #ifdef __EMSCRIPTEN__ - int is_runtime_thread = emscripten_is_main_runtime_thread(); + const int is_runtime_thread = emscripten_is_main_runtime_thread(); while (inst->finished == 1) { if (is_runtime_thread) { int e; do { - // Main thread waits in _very_ small slices so that it stays responsive to assist proxied - // pthread calls. - e = emscripten_futex_wait(&inst->finished, 1, 1); // Assist other threads by executing proxied operations that are effectively singlethreaded. emscripten_main_thread_process_queued_calls(); - } while(e == -ETIMEDOUT); + // Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. + e = emscripten_futex_wait(&inst->finished, 1, 1); + } while (e == -ETIMEDOUT); } else { // Can wait in one go. emscripten_futex_wait(&inst->finished, 1, INFINITY); diff --git a/system/lib/pthread/library_pthread.c b/system/lib/pthread/library_pthread.c index 80e3fa193c249..18c87530b8f83 100644 --- a/system/lib/pthread/library_pthread.c +++ b/system/lib/pthread/library_pthread.c @@ -87,35 +87,29 @@ void emscripten_thread_sleep(double msecs) { double now = emscripten_get_now(); double target = now + msecs; - __pthread_testcancel(); // pthreads spec: sleep is a cancellation point, so must test if this - // thread is cancelled during the sleep. - emscripten_current_thread_process_queued_calls(); - // If we have less than this many msecs left to wait, busy spin that instead. - double min_ms_slice_to_sleep = 0.1; + const double min_ms_slice_to_sleep = 0.1; - // runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. - double max_ms_slice_to_sleep = emscripten_is_main_runtime_thread() ? 1 : 100; + // Runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. + const double max_ms_slice_to_sleep = emscripten_is_main_runtime_thread() ? 1 : 100; emscripten_conditional_set_current_thread_status( EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_SLEEPING); - now = emscripten_get_now(); - while (now < target) { + + double ms_to_sleep; + do { // Keep processing the main loop of the calling thread. __pthread_testcancel(); // pthreads spec: sleep is a cancellation point, so must test if this // thread is cancelled during the sleep. emscripten_current_thread_process_queued_calls(); - now = emscripten_get_now(); - double ms_to_sleep = target - now; - if (ms_to_sleep > max_ms_slice_to_sleep) { - ms_to_sleep = max_ms_slice_to_sleep; - } - if (ms_to_sleep >= min_ms_slice_to_sleep) { - emscripten_futex_wait(&dummyZeroAddress, 0, ms_to_sleep); - } - now = emscripten_get_now(); - }; + ms_to_sleep = target - emscripten_get_now(); + if (ms_to_sleep < min_ms_slice_to_sleep) + continue; + + emscripten_futex_wait(&dummyZeroAddress, 0, + ms_to_sleep > max_ms_slice_to_sleep ? max_ms_slice_to_sleep : ms_to_sleep); + } while (ms_to_sleep > 0); emscripten_conditional_set_current_thread_status( EM_THREAD_STATUS_SLEEPING, EM_THREAD_STATUS_RUNNING); @@ -390,24 +384,22 @@ static CallQueue* GetOrAllocateQueue(void* target) { } EMSCRIPTEN_RESULT emscripten_wait_for_call_v(em_queued_call* call, double timeoutMSecs) { - int r; - int done = atomic_load(&call->operationDone); - if (!done) { - double now = emscripten_get_now(); - double waitEndTime = now + timeoutMSecs; - emscripten_set_current_thread_status(EM_THREAD_STATUS_WAITPROXY); - while (!done && now < waitEndTime) { - r = emscripten_futex_wait(&call->operationDone, 0, waitEndTime - now); - done = atomic_load(&call->operationDone); - now = emscripten_get_now(); - } - emscripten_set_current_thread_status(EM_THREAD_STATUS_RUNNING); - } - if (done) - return EMSCRIPTEN_RESULT_SUCCESS; - else - return EMSCRIPTEN_RESULT_TIMED_OUT; + if (done) return EMSCRIPTEN_RESULT_SUCCESS; + + emscripten_set_current_thread_status(EM_THREAD_STATUS_WAITPROXY); + + double timeoutUntilTime = emscripten_get_now() + timeoutMSecs; + do { + emscripten_futex_wait(&call->operationDone, 0, timeoutMSecs); + done = atomic_load(&call->operationDone); + + timeoutMSecs = timeoutUntilTime - emscripten_get_now(); + } while (!done && timeoutMSecs > 0); + + emscripten_set_current_thread_status(EM_THREAD_STATUS_RUNNING); + + return done ? EMSCRIPTEN_RESULT_SUCCESS : EMSCRIPTEN_RESULT_TIMED_OUT; } EMSCRIPTEN_RESULT emscripten_wait_for_call_i( From c0b7882ff55a3cf0cac32686fd9882cde9fb46c2 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Fri, 10 Dec 2021 17:29:27 +0100 Subject: [PATCH 02/14] Revert unnecessary changes --- system/lib/libc/musl/src/thread/__timedwait.c | 9 +++------ system/lib/libc/musl/src/thread/__wait.c | 2 -- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/system/lib/libc/musl/src/thread/__timedwait.c b/system/lib/libc/musl/src/thread/__timedwait.c index 15b51b4e8a0ef..18e5a647d0f57 100644 --- a/system/lib/libc/musl/src/thread/__timedwait.c +++ b/system/lib/libc/musl/src/thread/__timedwait.c @@ -32,10 +32,10 @@ static int __futex4_cp(volatile void *addr, int op, int val, const struct timesp if (r != -ENOSYS) return r; return __syscall_cp(SYS_futex, addr, op & ~FUTEX_PRIVATE, val, to); } +#endif static volatile int dummy = 0; weak_alias(dummy, __eintr_valid_flag); -#endif int __timedwait_cp(volatile int *addr, int val, clockid_t clk, const struct timespec *at, int priv) @@ -43,9 +43,7 @@ int __timedwait_cp(volatile int *addr, int val, int r; struct timespec to, *top=0; -#ifndef __EMSCRIPTEN__ if (priv) priv = FUTEX_PRIVATE; -#endif if (at) { if (at->tv_nsec >= 1000000000UL) return EINVAL; @@ -58,9 +56,10 @@ int __timedwait_cp(volatile int *addr, int val, if (to.tv_sec < 0) return ETIMEDOUT; top = &to; } + #ifdef __EMSCRIPTEN__ pthread_t self = __pthread_self(); - double msecsToSleep = top ? (top->tv_sec * 1000.0 + top->tv_nsec / 1000000.0) : INFINITY; + double msecsToSleep = top ? (top->tv_sec * 1000 + top->tv_nsec / 1000000.0) : INFINITY; const int is_runtime_thread = emscripten_is_main_runtime_thread(); // Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. @@ -94,13 +93,11 @@ int __timedwait_cp(volatile int *addr, int val, r = -__futex4_cp(addr, FUTEX_WAIT|priv, val, top); #endif if (r != EINTR && r != ETIMEDOUT && r != ECANCELED) r = 0; -#ifndef __EMSCRIPTEN__ // XXX Emscripten revert musl commit a63c0104e496f7ba78b64be3cd299b41e8cd427f /* Mitigate bug in old kernels wrongly reporting EINTR for non- * interrupting (SA_RESTART) signal handlers. This is only practical * when NO interrupting signal handlers have been installed, and * works by sigaction tracking whether that's the case. */ if (r == EINTR && !__eintr_valid_flag) r = 0; -#endif return r; } diff --git a/system/lib/libc/musl/src/thread/__wait.c b/system/lib/libc/musl/src/thread/__wait.c index c5eca7935d627..fe80018cc4cb4 100644 --- a/system/lib/libc/musl/src/thread/__wait.c +++ b/system/lib/libc/musl/src/thread/__wait.c @@ -8,9 +8,7 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv) { int spins=100; -#ifndef __EMSCRIPTEN__ if (priv) priv = FUTEX_PRIVATE; -#endif while (spins-- && (!waiters || !*waiters)) { if (*addr==val) a_spin(); else return; From 21c0823c419a15b607f383d733d7c77cc4bfa2e1 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Fri, 10 Dec 2021 17:34:55 +0100 Subject: [PATCH 03/14] Revert hoisting of `self` changes --- system/lib/libc/musl/src/thread/__timedwait.c | 7 ++++--- system/lib/libc/musl/src/thread/__wait.c | 5 ++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/system/lib/libc/musl/src/thread/__timedwait.c b/system/lib/libc/musl/src/thread/__timedwait.c index 18e5a647d0f57..a7acacd97b734 100644 --- a/system/lib/libc/musl/src/thread/__timedwait.c +++ b/system/lib/libc/musl/src/thread/__timedwait.c @@ -58,7 +58,6 @@ int __timedwait_cp(volatile int *addr, int val, } #ifdef __EMSCRIPTEN__ - pthread_t self = __pthread_self(); double msecsToSleep = top ? (top->tv_sec * 1000 + top->tv_nsec / 1000000.0) : INFINITY; const int is_runtime_thread = emscripten_is_main_runtime_thread(); @@ -68,10 +67,12 @@ int __timedwait_cp(volatile int *addr, int val, // cp suffix in the function name means "cancellation point", so this wait can be cancelled // by the users unless current threads cancellability is set to PTHREAD_CANCEL_DISABLE // which may be either done by the user of __timedwait() function. - if (is_runtime_thread || self->canceldisable != PTHREAD_CANCEL_DISABLE || self->cancelasync) { + if (is_runtime_thread || + pthread_self()->canceldisable != PTHREAD_CANCEL_DISABLE || + pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) { double sleepUntilTime = emscripten_get_now() + msecsToSleep; do { - if (self->cancel) { + if (pthread_self()->cancel) { // Emscripten-specific return value: The wait was canceled by user calling // pthread_cancel() for this thread, and the caller needs to cooperatively // cancel execution. diff --git a/system/lib/libc/musl/src/thread/__wait.c b/system/lib/libc/musl/src/thread/__wait.c index fe80018cc4cb4..fff29f096f8a1 100644 --- a/system/lib/libc/musl/src/thread/__wait.c +++ b/system/lib/libc/musl/src/thread/__wait.c @@ -15,17 +15,16 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv) } if (waiters) a_inc(waiters); #ifdef __EMSCRIPTEN__ - pthread_t self = __pthread_self(); const int is_runtime_thread = emscripten_is_main_runtime_thread(); // Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. const double maxMsecsToSleep = is_runtime_thread ? 1 : 100; while (*addr==val) { - if (is_runtime_thread || self->cancelasync) { + if (is_runtime_thread || pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) { int e; do { - if (self->cancel) { + if (pthread_self()->cancel) { if (waiters) a_dec(waiters); return; } From e9692af3d1cb86a3489d0445f53360f48805377c Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Fri, 10 Dec 2021 17:40:30 +0100 Subject: [PATCH 04/14] Revert small doc changes --- system/lib/libc/musl/src/thread/__timedwait.c | 2 +- system/lib/pthread/library_pthread.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/system/lib/libc/musl/src/thread/__timedwait.c b/system/lib/libc/musl/src/thread/__timedwait.c index a7acacd97b734..382a1f4b7c773 100644 --- a/system/lib/libc/musl/src/thread/__timedwait.c +++ b/system/lib/libc/musl/src/thread/__timedwait.c @@ -65,7 +65,7 @@ int __timedwait_cp(volatile int *addr, int val, const double maxMsecsToSleep = is_runtime_thread ? 1 : 100; // cp suffix in the function name means "cancellation point", so this wait can be cancelled - // by the users unless current threads cancellability is set to PTHREAD_CANCEL_DISABLE + // by the users unless current threads cancelability is set to PTHREAD_CANCEL_DISABLE // which may be either done by the user of __timedwait() function. if (is_runtime_thread || pthread_self()->canceldisable != PTHREAD_CANCEL_DISABLE || diff --git a/system/lib/pthread/library_pthread.c b/system/lib/pthread/library_pthread.c index 18c87530b8f83..a3ae64bcac8bc 100644 --- a/system/lib/pthread/library_pthread.c +++ b/system/lib/pthread/library_pthread.c @@ -90,7 +90,7 @@ void emscripten_thread_sleep(double msecs) { // If we have less than this many msecs left to wait, busy spin that instead. const double min_ms_slice_to_sleep = 0.1; - // Runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. + // runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. const double max_ms_slice_to_sleep = emscripten_is_main_runtime_thread() ? 1 : 100; emscripten_conditional_set_current_thread_status( From 2a525980cccb2a352f00461589a9b48ad76ee172 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Fri, 10 Dec 2021 17:41:18 +0100 Subject: [PATCH 05/14] Drop const for changed lines --- system/lib/libc/musl/src/thread/__timedwait.c | 4 ++-- system/lib/libc/musl/src/thread/__wait.c | 4 ++-- system/lib/libc/musl/src/thread/pthread_barrier_wait.c | 2 +- system/lib/pthread/library_pthread.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/system/lib/libc/musl/src/thread/__timedwait.c b/system/lib/libc/musl/src/thread/__timedwait.c index 382a1f4b7c773..94591c5bb3136 100644 --- a/system/lib/libc/musl/src/thread/__timedwait.c +++ b/system/lib/libc/musl/src/thread/__timedwait.c @@ -59,10 +59,10 @@ int __timedwait_cp(volatile int *addr, int val, #ifdef __EMSCRIPTEN__ double msecsToSleep = top ? (top->tv_sec * 1000 + top->tv_nsec / 1000000.0) : INFINITY; - const int is_runtime_thread = emscripten_is_main_runtime_thread(); + int is_runtime_thread = emscripten_is_main_runtime_thread(); // Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. - const double maxMsecsToSleep = is_runtime_thread ? 1 : 100; + double maxMsecsToSleep = is_runtime_thread ? 1 : 100; // cp suffix in the function name means "cancellation point", so this wait can be cancelled // by the users unless current threads cancelability is set to PTHREAD_CANCEL_DISABLE diff --git a/system/lib/libc/musl/src/thread/__wait.c b/system/lib/libc/musl/src/thread/__wait.c index fff29f096f8a1..df4ae4ce7a043 100644 --- a/system/lib/libc/musl/src/thread/__wait.c +++ b/system/lib/libc/musl/src/thread/__wait.c @@ -15,10 +15,10 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv) } if (waiters) a_inc(waiters); #ifdef __EMSCRIPTEN__ - const int is_runtime_thread = emscripten_is_main_runtime_thread(); + int is_runtime_thread = emscripten_is_main_runtime_thread(); // Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. - const double maxMsecsToSleep = is_runtime_thread ? 1 : 100; + double maxMsecsToSleep = is_runtime_thread ? 1 : 100; while (*addr==val) { if (is_runtime_thread || pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) { diff --git a/system/lib/libc/musl/src/thread/pthread_barrier_wait.c b/system/lib/libc/musl/src/thread/pthread_barrier_wait.c index a254bbccdcd5d..9474476986e25 100644 --- a/system/lib/libc/musl/src/thread/pthread_barrier_wait.c +++ b/system/lib/libc/musl/src/thread/pthread_barrier_wait.c @@ -88,7 +88,7 @@ int pthread_barrier_wait(pthread_barrier_t *b) a_spin(); a_inc(&inst->finished); #ifdef __EMSCRIPTEN__ - const int is_runtime_thread = emscripten_is_main_runtime_thread(); + int is_runtime_thread = emscripten_is_main_runtime_thread(); while (inst->finished == 1) { if (is_runtime_thread) { int e; diff --git a/system/lib/pthread/library_pthread.c b/system/lib/pthread/library_pthread.c index a3ae64bcac8bc..d2f7384d014be 100644 --- a/system/lib/pthread/library_pthread.c +++ b/system/lib/pthread/library_pthread.c @@ -88,10 +88,10 @@ void emscripten_thread_sleep(double msecs) { double target = now + msecs; // If we have less than this many msecs left to wait, busy spin that instead. - const double min_ms_slice_to_sleep = 0.1; + double min_ms_slice_to_sleep = 0.1; // runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. - const double max_ms_slice_to_sleep = emscripten_is_main_runtime_thread() ? 1 : 100; + double max_ms_slice_to_sleep = emscripten_is_main_runtime_thread() ? 1 : 100; emscripten_conditional_set_current_thread_status( EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_SLEEPING); From 4b8a02431bf43403c3aba04041eddb9c1ca5df1d Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Fri, 10 Dec 2021 17:48:25 +0100 Subject: [PATCH 06/14] Prefer snake_case for changed lines --- system/lib/libc/musl/src/thread/__timedwait.c | 4 ++-- system/lib/libc/musl/src/thread/__wait.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/system/lib/libc/musl/src/thread/__timedwait.c b/system/lib/libc/musl/src/thread/__timedwait.c index 94591c5bb3136..754c46545e0d0 100644 --- a/system/lib/libc/musl/src/thread/__timedwait.c +++ b/system/lib/libc/musl/src/thread/__timedwait.c @@ -62,7 +62,7 @@ int __timedwait_cp(volatile int *addr, int val, int is_runtime_thread = emscripten_is_main_runtime_thread(); // Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. - double maxMsecsToSleep = is_runtime_thread ? 1 : 100; + double max_ms_to_sleep = is_runtime_thread ? 1 : 100; // cp suffix in the function name means "cancellation point", so this wait can be cancelled // by the users unless current threads cancelability is set to PTHREAD_CANCEL_DISABLE @@ -80,7 +80,7 @@ int __timedwait_cp(volatile int *addr, int val, } // Must wait in slices in case this thread is cancelled in between. r = -emscripten_futex_wait((void*)addr, val, - msecsToSleep > maxMsecsToSleep ? maxMsecsToSleep : msecsToSleep); + msecsToSleep > max_ms_to_sleep ? max_ms_to_sleep : msecsToSleep); // Assist other threads by executing proxied operations that are effectively singlethreaded. if (is_runtime_thread) emscripten_main_thread_process_queued_calls(); diff --git a/system/lib/libc/musl/src/thread/__wait.c b/system/lib/libc/musl/src/thread/__wait.c index df4ae4ce7a043..03c95f904463d 100644 --- a/system/lib/libc/musl/src/thread/__wait.c +++ b/system/lib/libc/musl/src/thread/__wait.c @@ -18,7 +18,7 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv) int is_runtime_thread = emscripten_is_main_runtime_thread(); // Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. - double maxMsecsToSleep = is_runtime_thread ? 1 : 100; + double max_ms_to_sleep = is_runtime_thread ? 1 : 100; while (*addr==val) { if (is_runtime_thread || pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) { @@ -29,7 +29,7 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv) return; } // Must wait in slices in case this thread is cancelled in between. - e = emscripten_futex_wait((void*)addr, val, maxMsecsToSleep); + e = emscripten_futex_wait((void*)addr, val, max_ms_to_sleep); // Assist other threads by executing proxied operations that are effectively singlethreaded. if (is_runtime_thread) emscripten_main_thread_process_queued_calls(); } while (e == -ETIMEDOUT); From 961e320129015a9737f18638a31c3885e5cb2c13 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Fri, 10 Dec 2021 18:15:59 +0100 Subject: [PATCH 07/14] Revert `emscripten_wait_for_call_v` change --- system/lib/pthread/library_pthread.c | 32 +++++++++++++++------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/system/lib/pthread/library_pthread.c b/system/lib/pthread/library_pthread.c index d2f7384d014be..6a24f4917f379 100644 --- a/system/lib/pthread/library_pthread.c +++ b/system/lib/pthread/library_pthread.c @@ -384,22 +384,24 @@ static CallQueue* GetOrAllocateQueue(void* target) { } EMSCRIPTEN_RESULT emscripten_wait_for_call_v(em_queued_call* call, double timeoutMSecs) { - int done = atomic_load(&call->operationDone); - if (done) return EMSCRIPTEN_RESULT_SUCCESS; - - emscripten_set_current_thread_status(EM_THREAD_STATUS_WAITPROXY); - - double timeoutUntilTime = emscripten_get_now() + timeoutMSecs; - do { - emscripten_futex_wait(&call->operationDone, 0, timeoutMSecs); - done = atomic_load(&call->operationDone); - - timeoutMSecs = timeoutUntilTime - emscripten_get_now(); - } while (!done && timeoutMSecs > 0); + int r; - emscripten_set_current_thread_status(EM_THREAD_STATUS_RUNNING); - - return done ? EMSCRIPTEN_RESULT_SUCCESS : EMSCRIPTEN_RESULT_TIMED_OUT; + int done = atomic_load(&call->operationDone); + if (!done) { + double now = emscripten_get_now(); + double waitEndTime = now + timeoutMSecs; + emscripten_set_current_thread_status(EM_THREAD_STATUS_WAITPROXY); + while (!done && now < waitEndTime) { + r = emscripten_futex_wait(&call->operationDone, 0, waitEndTime - now); + done = atomic_load(&call->operationDone); + now = emscripten_get_now(); + } + emscripten_set_current_thread_status(EM_THREAD_STATUS_RUNNING); + } + if (done) + return EMSCRIPTEN_RESULT_SUCCESS; + else + return EMSCRIPTEN_RESULT_TIMED_OUT; } EMSCRIPTEN_RESULT emscripten_wait_for_call_i( From 4784dfaa77e75c4edd7bca3957794b5ef5307c88 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Fri, 10 Dec 2021 18:18:52 +0100 Subject: [PATCH 08/14] Consolidate vars --- system/lib/pthread/library_pthread.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/system/lib/pthread/library_pthread.c b/system/lib/pthread/library_pthread.c index 6a24f4917f379..a71911a824451 100644 --- a/system/lib/pthread/library_pthread.c +++ b/system/lib/pthread/library_pthread.c @@ -84,8 +84,7 @@ int pthread_mutexattr_setprioceiling(pthread_mutexattr_t *attr, int prioceiling) static uint32_t dummyZeroAddress = 0; void emscripten_thread_sleep(double msecs) { - double now = emscripten_get_now(); - double target = now + msecs; + double target = emscripten_get_now() + msecs; // If we have less than this many msecs left to wait, busy spin that instead. double min_ms_slice_to_sleep = 0.1; From 02ba5d67a51724e55dd58fc1b17a7a1dab711552 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Thu, 13 Jan 2022 11:59:15 +0100 Subject: [PATCH 09/14] s/max_ms_to_sleep/max_ms_slice_to_sleep/g --- system/lib/libc/musl/src/thread/__timedwait.c | 4 ++-- system/lib/libc/musl/src/thread/__wait.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/system/lib/libc/musl/src/thread/__timedwait.c b/system/lib/libc/musl/src/thread/__timedwait.c index 754c46545e0d0..b411f7d580750 100644 --- a/system/lib/libc/musl/src/thread/__timedwait.c +++ b/system/lib/libc/musl/src/thread/__timedwait.c @@ -62,7 +62,7 @@ int __timedwait_cp(volatile int *addr, int val, int is_runtime_thread = emscripten_is_main_runtime_thread(); // Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. - double max_ms_to_sleep = is_runtime_thread ? 1 : 100; + double max_ms_slice_to_sleep = is_runtime_thread ? 1 : 100; // cp suffix in the function name means "cancellation point", so this wait can be cancelled // by the users unless current threads cancelability is set to PTHREAD_CANCEL_DISABLE @@ -80,7 +80,7 @@ int __timedwait_cp(volatile int *addr, int val, } // Must wait in slices in case this thread is cancelled in between. r = -emscripten_futex_wait((void*)addr, val, - msecsToSleep > max_ms_to_sleep ? max_ms_to_sleep : msecsToSleep); + msecsToSleep > max_ms_slice_to_sleep ? max_ms_slice_to_sleep : msecsToSleep); // Assist other threads by executing proxied operations that are effectively singlethreaded. if (is_runtime_thread) emscripten_main_thread_process_queued_calls(); diff --git a/system/lib/libc/musl/src/thread/__wait.c b/system/lib/libc/musl/src/thread/__wait.c index 03c95f904463d..d42469a1bc320 100644 --- a/system/lib/libc/musl/src/thread/__wait.c +++ b/system/lib/libc/musl/src/thread/__wait.c @@ -18,7 +18,7 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv) int is_runtime_thread = emscripten_is_main_runtime_thread(); // Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. - double max_ms_to_sleep = is_runtime_thread ? 1 : 100; + double max_ms_slice_to_sleep = is_runtime_thread ? 1 : 100; while (*addr==val) { if (is_runtime_thread || pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) { @@ -29,7 +29,7 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv) return; } // Must wait in slices in case this thread is cancelled in between. - e = emscripten_futex_wait((void*)addr, val, max_ms_to_sleep); + e = emscripten_futex_wait((void*)addr, val, max_ms_slice_to_sleep); // Assist other threads by executing proxied operations that are effectively singlethreaded. if (is_runtime_thread) emscripten_main_thread_process_queued_calls(); } while (e == -ETIMEDOUT); From c8d47d03d9437e67587ad7766184feb7119ec93a Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Thu, 13 Jan 2022 12:01:18 +0100 Subject: [PATCH 10/14] Avoid call to `emscripten_futex_wait` when `msecsToSleep <= 0` --- system/lib/libc/musl/src/thread/__timedwait.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/system/lib/libc/musl/src/thread/__timedwait.c b/system/lib/libc/musl/src/thread/__timedwait.c index b411f7d580750..608cf66dee05b 100644 --- a/system/lib/libc/musl/src/thread/__timedwait.c +++ b/system/lib/libc/musl/src/thread/__timedwait.c @@ -78,9 +78,10 @@ int __timedwait_cp(volatile int *addr, int val, // cancel execution. return ECANCELED; } - // Must wait in slices in case this thread is cancelled in between. - r = -emscripten_futex_wait((void*)addr, val, - msecsToSleep > max_ms_slice_to_sleep ? max_ms_slice_to_sleep : msecsToSleep); + if (msecsToSleep > 0) + // Must wait in slices in case this thread is cancelled in between. + r = -emscripten_futex_wait((void*)addr, val, + msecsToSleep > max_ms_slice_to_sleep ? max_ms_slice_to_sleep : msecsToSleep); // Assist other threads by executing proxied operations that are effectively singlethreaded. if (is_runtime_thread) emscripten_main_thread_process_queued_calls(); From 40064d65009c4282a7549097fb6c6e0f6dc661e4 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Sat, 15 Jan 2022 11:39:12 +0100 Subject: [PATCH 11/14] Resolve test failures By always calling `emscripten_main_thread_process_queued_calls()` before `emscripten_futex_wait()`. --- system/lib/libc/musl/src/thread/__timedwait.c | 13 ++++++++----- system/lib/libc/musl/src/thread/__wait.c | 4 ++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/system/lib/libc/musl/src/thread/__timedwait.c b/system/lib/libc/musl/src/thread/__timedwait.c index 608cf66dee05b..bb9250ea75636 100644 --- a/system/lib/libc/musl/src/thread/__timedwait.c +++ b/system/lib/libc/musl/src/thread/__timedwait.c @@ -78,15 +78,18 @@ int __timedwait_cp(volatile int *addr, int val, // cancel execution. return ECANCELED; } - if (msecsToSleep > 0) - // Must wait in slices in case this thread is cancelled in between. - r = -emscripten_futex_wait((void*)addr, val, - msecsToSleep > max_ms_slice_to_sleep ? max_ms_slice_to_sleep : msecsToSleep); // Assist other threads by executing proxied operations that are effectively singlethreaded. if (is_runtime_thread) emscripten_main_thread_process_queued_calls(); msecsToSleep = sleepUntilTime - emscripten_get_now(); - } while (r == ETIMEDOUT && msecsToSleep > 0); + if (msecsToSleep <= 0) { + r = ETIMEDOUT; + break; + } + // Must wait in slices in case this thread is cancelled in between. + r = -emscripten_futex_wait((void*)addr, val, + msecsToSleep > max_ms_slice_to_sleep ? max_ms_slice_to_sleep : msecsToSleep); + } while (r == ETIMEDOUT); } else { // Can wait in one go. r = -emscripten_futex_wait((void*)addr, val, msecsToSleep); diff --git a/system/lib/libc/musl/src/thread/__wait.c b/system/lib/libc/musl/src/thread/__wait.c index d42469a1bc320..2966ee221254f 100644 --- a/system/lib/libc/musl/src/thread/__wait.c +++ b/system/lib/libc/musl/src/thread/__wait.c @@ -28,10 +28,10 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv) if (waiters) a_dec(waiters); return; } - // Must wait in slices in case this thread is cancelled in between. - e = emscripten_futex_wait((void*)addr, val, max_ms_slice_to_sleep); // Assist other threads by executing proxied operations that are effectively singlethreaded. if (is_runtime_thread) emscripten_main_thread_process_queued_calls(); + // Must wait in slices in case this thread is cancelled in between. + e = emscripten_futex_wait((void*)addr, val, max_ms_slice_to_sleep); } while (e == -ETIMEDOUT); } else { // Can wait in one go. From 70bfc3b41bc996e4c6125130f2800c62072cdce5 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Mon, 17 Jan 2022 18:34:24 +0100 Subject: [PATCH 12/14] Split ternary operation over 2 lines Improves readability. --- system/lib/libc/musl/src/thread/__timedwait.c | 5 +++-- system/lib/pthread/library_pthread.c | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/system/lib/libc/musl/src/thread/__timedwait.c b/system/lib/libc/musl/src/thread/__timedwait.c index bb9250ea75636..c143ac8887c7e 100644 --- a/system/lib/libc/musl/src/thread/__timedwait.c +++ b/system/lib/libc/musl/src/thread/__timedwait.c @@ -87,8 +87,9 @@ int __timedwait_cp(volatile int *addr, int val, break; } // Must wait in slices in case this thread is cancelled in between. - r = -emscripten_futex_wait((void*)addr, val, - msecsToSleep > max_ms_slice_to_sleep ? max_ms_slice_to_sleep : msecsToSleep); + if (msecsToSleep > max_ms_slice_to_sleep) + msecsToSleep = max_ms_slice_to_sleep; + r = -emscripten_futex_wait((void*)addr, val, msecsToSleep); } while (r == ETIMEDOUT); } else { // Can wait in one go. diff --git a/system/lib/pthread/library_pthread.c b/system/lib/pthread/library_pthread.c index a71911a824451..90e6358b7bd9d 100644 --- a/system/lib/pthread/library_pthread.c +++ b/system/lib/pthread/library_pthread.c @@ -105,9 +105,9 @@ void emscripten_thread_sleep(double msecs) { ms_to_sleep = target - emscripten_get_now(); if (ms_to_sleep < min_ms_slice_to_sleep) continue; - - emscripten_futex_wait(&dummyZeroAddress, 0, - ms_to_sleep > max_ms_slice_to_sleep ? max_ms_slice_to_sleep : ms_to_sleep); + if (ms_to_sleep > max_ms_slice_to_sleep) + ms_to_sleep = max_ms_slice_to_sleep; + emscripten_futex_wait(&dummyZeroAddress, 0, ms_to_sleep); } while (ms_to_sleep > 0); emscripten_conditional_set_current_thread_status( From ecbd5fa2aee4ce4df1b6604af037b0cc23bd4bb6 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Thu, 20 Jan 2022 18:29:48 +0100 Subject: [PATCH 13/14] Revert non-NFC change As it's essentially a bug fix. --- system/lib/libc/musl/src/thread/pthread_barrier_wait.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system/lib/libc/musl/src/thread/pthread_barrier_wait.c b/system/lib/libc/musl/src/thread/pthread_barrier_wait.c index 9474476986e25..4ba11d579bd9a 100644 --- a/system/lib/libc/musl/src/thread/pthread_barrier_wait.c +++ b/system/lib/libc/musl/src/thread/pthread_barrier_wait.c @@ -93,10 +93,10 @@ int pthread_barrier_wait(pthread_barrier_t *b) if (is_runtime_thread) { int e; do { - // Assist other threads by executing proxied operations that are effectively singlethreaded. - emscripten_main_thread_process_queued_calls(); // Main runtime thread may need to run proxied calls, so sleep in very small slices to be responsive. e = emscripten_futex_wait(&inst->finished, 1, 1); + // Assist other threads by executing proxied operations that are effectively singlethreaded. + emscripten_main_thread_process_queued_calls(); } while (e == -ETIMEDOUT); } else { // Can wait in one go. From 7d74c5316dc8bd1b4e4df1eefb10cf375f73cb18 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Thu, 20 Jan 2022 20:40:54 +0100 Subject: [PATCH 14/14] Incorporate review comment --- system/lib/pthread/library_pthread.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/system/lib/pthread/library_pthread.c b/system/lib/pthread/library_pthread.c index 90e6358b7bd9d..3da6d4290896b 100644 --- a/system/lib/pthread/library_pthread.c +++ b/system/lib/pthread/library_pthread.c @@ -84,7 +84,8 @@ int pthread_mutexattr_setprioceiling(pthread_mutexattr_t *attr, int prioceiling) static uint32_t dummyZeroAddress = 0; void emscripten_thread_sleep(double msecs) { - double target = emscripten_get_now() + msecs; + double now = emscripten_get_now(); + double target = now + msecs; // If we have less than this many msecs left to wait, busy spin that instead. double min_ms_slice_to_sleep = 0.1; @@ -95,20 +96,21 @@ void emscripten_thread_sleep(double msecs) { emscripten_conditional_set_current_thread_status( EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_SLEEPING); - double ms_to_sleep; do { // Keep processing the main loop of the calling thread. __pthread_testcancel(); // pthreads spec: sleep is a cancellation point, so must test if this // thread is cancelled during the sleep. emscripten_current_thread_process_queued_calls(); - ms_to_sleep = target - emscripten_get_now(); + now = emscripten_get_now(); + double ms_to_sleep = target - now; if (ms_to_sleep < min_ms_slice_to_sleep) continue; if (ms_to_sleep > max_ms_slice_to_sleep) ms_to_sleep = max_ms_slice_to_sleep; emscripten_futex_wait(&dummyZeroAddress, 0, ms_to_sleep); - } while (ms_to_sleep > 0); + now = emscripten_get_now(); + } while (now < target); emscripten_conditional_set_current_thread_status( EM_THREAD_STATUS_SLEEPING, EM_THREAD_STATUS_RUNNING);