Remove loop from emscripten_thread_sleep. NFC (#26585)

We have code in the lower level `emscrpeten_futex_wait` that takes care
of splitting up the wait for the main runtime thread.

For normal pthreads I don't see any need to run the message queue
constantly while the thread is sleeping.

As part of this change I found that I had to improve the accuracy of the
timeslicing `emscripten_futex_wait`.
The new code uses an absolute target time, so regardless of how much
time it spends between calls to
`atomic.wait` it still returns at the correct time.

Previously we were assuming that we could call `atomic.wait32` with 1ms
timeout N times to achieve Nms of sleeping, but this doesn't always
hold, for various reasons.

Followup to #26471

Author: sbc100

system/lib/pthread/emscripten_futex_wait.c

@@ -172,15 +172,15 @@ int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms)
     wakeup_interval = 100 * 1000000;
   }
 
-  // When wakeup_interval is set, we use remainder_ns to track how many ns
-  // remain of the intiial max_wait_ns.
-  int64_t remainder_ns = 0;
+  // When wakeup_interval is set, we use end_time to track the absolute
+  // time when the wait should end.
+  double end_time = 0;
   if (wakeup_interval) {
-    remainder_ns = max_wait_ns;
-    if (remainder_ns < 0) {
+    if (max_wait_ms == INFINITY) {
       max_wait_ns = wakeup_interval;
     } else {
-      max_wait_ns = MIN(remainder_ns, wakeup_interval);
+      end_time = emscripten_get_now() + max_wait_ms;
+      max_wait_ns = MIN(max_wait_ns, wakeup_interval);
     }
   }
 
@@ -222,14 +222,12 @@ int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms)
       emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_WAITFUTEX, EM_THREAD_STATUS_RUNNING);
       return -ECANCELED;
     }
-    // If remainder_ns is negative it means we want wait forever, and we don't
-    // need to decrement remainder_ns in that case.
-    if (wakeup_interval && remainder_ns >= 0) {
-      remainder_ns -= wakeup_interval;
-      if (remainder_ns <= 0) {
+    if (wakeup_interval && max_wait_ms != INFINITY) {
+      double remainder_ms = end_time - emscripten_get_now();
+      if (remainder_ms <= 0) {
         break;
       }
-      max_wait_ns = MIN(remainder_ns, wakeup_interval);
+      max_wait_ns = MIN((int64_t)(remainder_ms * 1e6), wakeup_interval);
     }
   } while (wakeup_interval && ret == ATOMICS_WAIT_TIMED_OUT);
 #endif

system/lib/pthread/library_pthread.c

@@ -65,54 +65,32 @@ int sched_get_priority_min(int policy) {
     return 0;
 }
 
-int pthread_mutexattr_getprioceiling(const pthread_mutexattr_t *restrict attr, int *restrict prioceiling)
-{
+int pthread_mutexattr_getprioceiling(const pthread_mutexattr_t *restrict attr, int *restrict prioceiling) {
   // Not supported either in Emscripten or musl, return a faked value.
   if (prioceiling) *prioceiling = 99;
   return 0;
 }
 
-int pthread_mutexattr_setprioceiling(pthread_mutexattr_t *attr, int prioceiling)
-{
+int pthread_mutexattr_setprioceiling(pthread_mutexattr_t *attr, int prioceiling) {
   // Not supported either in Emscripten or musl, return an error.
   return EPERM;
 }
 
-static uint32_t dummyZeroAddress = 0;
-
 void emscripten_thread_sleep(double 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;
-
-  // Break up sleeping so that we process proxied work at regular intervals.
-  // TODO(sbc): This should be removed and/or moved down into
-  // `emscripten_futex_wait`.
-  double max_ms_slice_to_sleep = 100;
-
-  emscripten_conditional_set_current_thread_status(
-    EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_SLEEPING);
-
-  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 < 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);
-    now = emscripten_get_now();
-  } while (now < target);
-
-  emscripten_conditional_set_current_thread_status(
-    EM_THREAD_STATUS_SLEEPING, EM_THREAD_STATUS_RUNNING);
+  // We include emscripten_current_thread_process_queued_calls before and
+  // after sleeping since that is how we recieve "async" signals.
+  // We include __pthread_testcancel here becuase clock_nanosleep is
+  // a pthread cancelation point.
+  emscripten_current_thread_process_queued_calls();
+  __pthread_testcancel();
+  emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_RUNNING,
+                                                   EM_THREAD_STATUS_SLEEPING);
+  uint32_t dummyZeroAddress = 0;
+  emscripten_futex_wait(&dummyZeroAddress, 0, msecs);
+  emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_SLEEPING,
+                                                   EM_THREAD_STATUS_RUNNING);
+  emscripten_current_thread_process_queued_calls();
+  __pthread_testcancel();
 }
 
 static struct pthread __main_pthread;

system/lib/pthread/library_pthread_stub.c

@@ -231,7 +231,9 @@ int pthread_cancel(pthread_t thread) {
   return 0;
 }
 
-void pthread_testcancel() {}
+void __pthread_testcancel() {}
+
+weak_alias(__pthread_testcancel, pthread_testcancel);
 
 _Noreturn void __pthread_exit(void* status) {
    exit(0);

test/codesize/test_codesize_minimal_pthreads.json

@@ -1,10 +1,10 @@
 {
   "a.out.js": 7363,
   "a.out.js.gz": 3604,
-  "a.out.nodebug.wasm": 19046,
-  "a.out.nodebug.wasm.gz": 8821,
-  "total": 26409,
-  "total_gz": 12425,
+  "a.out.nodebug.wasm": 19003,
+  "a.out.nodebug.wasm.gz": 8786,
+  "total": 26366,
+  "total_gz": 12390,
   "sent": [
     "a (memory)",
     "b (exit)",

test/codesize/test_codesize_minimal_pthreads_memgrowth.json

@@ -1,10 +1,10 @@
 {
   "a.out.js": 7765,
   "a.out.js.gz": 3810,
-  "a.out.nodebug.wasm": 19047,
-  "a.out.nodebug.wasm.gz": 8822,
-  "total": 26812,
-  "total_gz": 12632,
+  "a.out.nodebug.wasm": 19004,
+  "a.out.nodebug.wasm.gz": 8787,
+  "total": 26769,
+  "total_gz": 12597,
   "sent": [
     "a (memory)",
     "b (exit)",

test/other/test_itimer.c

@@ -33,14 +33,15 @@ void prof_handler(int dummy) {
 }
 
 void test_oneoff(int which) {
+  printf("test_oneoff\n");
   memset(got_alarm, 0, sizeof(got_alarm));
 
   int rtn;
   struct itimerval val;
   memset(&val, 0, sizeof(val));
 
-  // Set a timer for 1 second
-  val.it_value.tv_sec = 1;
+  // Set a non-repeating timer for 500 milliseconds
+  val.it_value.tv_usec = 500 * 1000;
   rtn = setitimer(which, &val, NULL);
   assert(rtn == 0);
 
@@ -59,9 +60,9 @@ void test_oneoff(int which) {
   assert(val.it_value.tv_sec == 0);
   assert(val.it_value.tv_usec > 0);
 
-  // Wait 1.5s
+  // Wait 800ms
   assert(!got_alarm[which]);
-  usleep(1500 * 1000);
+  usleep(700 * 1000);
 
   // Verify that the time fired and is no longer active
   assert(got_alarm[which]);
@@ -74,6 +75,8 @@ void test_oneoff(int which) {
 #define ERROR_MARGIN 3
 
 void test_sequence(int which) {
+  int64_t ms_to_sleep = NUM_TIMERS * 100 + 50;
+  printf("test_sequence (sleeping for %lldms)\n", ms_to_sleep);
   memset(got_alarm, 0, sizeof(got_alarm));
   // Set a timer to fire every 100ms
   struct itimerval val;
@@ -83,7 +86,7 @@ void test_sequence(int which) {
   val.it_interval.tv_usec = 100 * 1000;
   int rtn = setitimer(which, &val, NULL);
   // Sleep for a little over NUM_TIMERS * 100ms
-  usleep((NUM_TIMERS * 100 + 50) * 1000);
+  usleep(ms_to_sleep * 1000);
   printf("got %d alarms\n", got_alarm[which]);
   // Normally we would expect NUM_TIMERS to fire in this time
   // but leave some wiggle room for scheduling anomalies.

test/pthread/test_pthread_kill.c

@@ -5,23 +5,26 @@
 
 #include <pthread.h>
 #include <sys/types.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <assert.h>
 #include <unistd.h>
 #include <errno.h>
 #include <signal.h>
 
+#include <emscripten/console.h>
+
 pthread_cond_t started_cond = PTHREAD_COND_INITIALIZER;
 pthread_mutex_t started_lock = PTHREAD_MUTEX_INITIALIZER;
-_Atomic int got_term_signal = 0;
+_Atomic bool got_term_signal = false;
 
 pthread_t thr;
 
 void signal_handler(int sig, siginfo_t * info, void * arg) {
   printf("signal: %d onthread=%d\n", sig, pthread_self() == thr);
   if (sig == SIGTERM) {
-    got_term_signal = 1;
+    got_term_signal = true;
   }
 }
 
@@ -63,9 +66,9 @@ int main() {
   printf("thread has started, sending SIGTERM\n");
 
   s = pthread_kill(thr, SIGTERM);
+  assert(s == 0);
   printf("SIGTERM sent\n");
 
-  assert(s == 0);
 
   pthread_join(thr, NULL);
   return 0;