Move sleep slicing from musl's __wait to the lower level emscripten_futex_wait. NFC

This means we only need to do this breaking up on our `wait` operations
in a single place.  It also means that other users the
`emscripten_futex_wait` API don't break pthread proxying or async
cancellation.

The moved code is only included in pthread-enabled builds so should not
effect Wasm Workers builders.

This change also paves the way for enabling musl's `__wait` to work
with `WASM_WORKERS`.

Author: sbc100

system/lib/libc/musl/src/thread/__wait.c

@@ -15,26 +15,11 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv)
 	}
 	if (waiters) a_inc(waiters);
 #ifdef __EMSCRIPTEN__
-	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_slice_to_sleep = is_runtime_thread ? 1 : 100;
-
-	while (*addr==val) {
-		if (is_runtime_thread || pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) {
-			int e;
-			do {
-				if (pthread_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, max_ms_slice_to_sleep);
-			} while (e == -ETIMEDOUT);
-		} else {
-			// Can wait in one go.
-			emscripten_futex_wait((void*)addr, val, INFINITY);
-		}
+	// loop here to handler spurious wakeups from the underlying
+	// emscripten_futex_wait.
+	int ret = 0;
+	while (*addr==val && ret == 0) {
+		ret = emscripten_futex_wait((void*)addr, val, INFINITY);
 	}
 #else
 	while (*addr==val) {

system/lib/pthread/emscripten_futex_wait.c

@@ -19,7 +19,7 @@ extern void* _emscripten_main_thread_futex;
 
 static int futex_wait_main_browser_thread(volatile void* addr,
                                           uint32_t val,
-                                          double timeout) {
+                                          double timeout, bool cancelable) {
   // Atomics.wait is not available in the main browser thread, so simulate it
   // via busy spinning. Only the main browser thread is allowed to call into
   // this function. It is not thread-safe to be called from any other thread.
@@ -45,6 +45,12 @@ static int futex_wait_main_browser_thread(volatile void* addr,
   assert(last_addr == 0);
 
   while (1) {
+#ifdef __EMSCRIPTEN_PTHREADS__
+    // We if we were cancelled
+    if (cancelable && pthread_self()->cancel) {
+      return -ETIMEDOUT;
+    }
+#endif
     // Check for a timeout.
     now = emscripten_get_now();
     if (now > end) {
@@ -114,68 +120,113 @@ static int futex_wait_main_browser_thread(volatile void* addr,
   return 0;
 }
 
+// memory.atomic.wait32 returns:
+//   0 => "ok", woken by another agent.
+//   1 => "not-equal", loaded value != expected value
+//   2 => "timed-out", the timeout expired
+#define ATOMIC_WAIT_OK 0
+#define ATOMIC_WAIT_NOT_EQUAL 1
+#define ATOMIC_WAIT_TIMEOUT 2
+
 int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms) {
   if ((((intptr_t)addr)&3) != 0) {
     return -EINVAL;
   }
 
-  // Pass 0 here, which means we don't have access to the current time in this
-  // function.  This tells _emscripten_yield to call emscripten_get_now if (and
-  // only if) it needs to know the time.
-  _emscripten_yield(0);
-
   int ret;
   emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_WAITFUTEX);
 
+#ifdef __EMSCRIPTEN_PTHREADS__
+  bool cancelable = pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS;
+#else
+  bool cancelable = false;
+#endif
+
   // For the main browser thread and audio worklets we can't use
   // __builtin_wasm_memory_atomic_wait32 so we have busy wait instead.
   if (!_emscripten_thread_supports_atomics_wait()) {
-    ret = futex_wait_main_browser_thread(addr, val, max_wait_ms);
+    ret = futex_wait_main_browser_thread(addr, val, max_wait_ms, cancelable);
     emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_WAITFUTEX, EM_THREAD_STATUS_RUNNING);
     return ret;
   }
 
   // -1 (or any negative number) means wait indefinitely.
   int64_t max_wait_ns = -1;
   if (max_wait_ms != INFINITY) {
-    max_wait_ns = (int64_t)(max_wait_ms*1000*1000);
+    max_wait_ns = (int64_t)(max_wait_ms * 1e6);
   }
-#ifdef EMSCRIPTEN_DYNAMIC_LINKING
-  // After the main thread queues dlopen events, it checks if the target threads
-  // are sleeping.
-  // If `sleeping` is set then the main thread knows that event will be
-  // processed after the sleep (before any other user code).  In this case the
-  // main thread does not wait for any kind of response form the thread.
-  // If `sleeping` is not set then we know we should wait for the thread process
-  // the queue, either from the call here directly after setting `sleeping` to
-  // 1, or from another callsite (e.g. the one in `emscripten_yield`).
-  int is_runtime_thread = emscripten_is_main_runtime_thread();
-  if (!is_runtime_thread) {
-    __pthread_self()->sleeping = 1;
-    _emscripten_process_dlopen_queue();
+
+#ifdef __EMSCRIPTEN_PTHREADS__
+  // When building with pthread support there are two conditions underwhich we
+  // need to limit the amount of time we spend in atomic.wait.
+  // 1. We are the main runtime thread.  In this case we need to be able to
+  //    process proxied events from workers.  Note that this is not always
+  //    the same as being the main browser thread.  For example, when running
+  //    under node or when launching and emscirpten-built program in a Web
+  //    Worker.  This this case we limit our wait slices to 1ms intervals.
+  // 2. When the current thread has async cancelation enabled.  In this case
+  //    we limit the wait duration to 100ms intervals.
+  int64_t wakeup_interval = 0;
+  bool is_runtime_thread = emscripten_is_main_runtime_thread();
+  if (is_runtime_thread) {
+    // If the current thread is the main runtime theead then only wait in 1ms slices.
+    wakeup_interval = 1 * 1e6;
+  }
+  else if (cancelable) {
+    // If the current thread is async cancelable then only wait in 100ms slices.
+    wakeup_interval = 100 * 1e6;
+  }
+
+  int64_t interations;
+  if (wakeup_interval) {
+    interations = max_wait_ns / wakeup_interval;
+    max_wait_ns = wakeup_interval;
   }
+
+  do {
 #endif
-  ret = __builtin_wasm_memory_atomic_wait32((int*)addr, val, max_wait_ns);
+    // Pass 0 here, which means we don't have access to the current time in this
+    // function.  This tells _emscripten_yield to call emscripten_get_now if (and
+    // only if) it needs to know the time.
+    _emscripten_yield(0);
+
 #ifdef EMSCRIPTEN_DYNAMIC_LINKING
-  if (!is_runtime_thread) {
-    __pthread_self()->sleeping = 0;
-    _emscripten_process_dlopen_queue();
-  }
+    // After the main thread queues dlopen events, it checks if the target threads
+    // are sleeping.
+    // If `sleeping` is set then the main thread knows that event will be
+    // processed after the sleep (before any other user code).  In this case the
+    // main thread does not wait for any kind of response form the thread.
+    // If `sleeping` is not set then we know we should wait for the thread process
+    // the queue, either from the call here directly after setting `sleeping` to
+    // 1, or from another callsite (e.g. the one in `emscripten_yield`).
+    if (!is_runtime_thread) {
+      __pthread_self()->sleeping = 1;
+      _emscripten_process_dlopen_queue();
+    }
+#endif
+    ret = __builtin_wasm_memory_atomic_wait32((int*)addr, val, max_wait_ns);
+#ifdef EMSCRIPTEN_DYNAMIC_LINKING
+    if (!is_runtime_thread) {
+      __pthread_self()->sleeping = 0;
+      _emscripten_process_dlopen_queue();
+    }
+#endif
+#ifdef __EMSCRIPTEN_PTHREADS__
+    if (cancelable && ret == ATOMIC_WAIT_TIMEOUT && pthread_self()->cancel) {
+      // We were cancelled
+      break;
+    }
+  } while (wakeup_interval && ret == ATOMIC_WAIT_TIMEOUT && (max_wait_ms == INFINITY || interations--));
 #endif
 
-done:
   emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_WAITFUTEX, EM_THREAD_STATUS_RUNNING);
 
-  // memory.atomic.wait32 returns:
-  //   0 => "ok", woken by another agent.
-  //   1 => "not-equal", loaded value != expected value
-  //   2 => "timed-out", the timeout expired
-  if (ret == 1) {
+  if (ret == ATOMIC_WAIT_NOT_EQUAL) {
     return -EWOULDBLOCK;
   }
-  if (ret == 2) {
+  if (ret == ATOMIC_WAIT_TIMEOUT) {
     return -ETIMEDOUT;
   }
-  assert(ret == 0);
+  assert(ret == ATOMIC_WAIT_OK);
   return 0;
 }

system/lib/pthread/library_pthread.c

@@ -87,8 +87,10 @@ void emscripten_thread_sleep(double msecs) {
   // If we have less than this many msecs left to wait, busy spin that instead.
   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;
+  // Break up sleeping so that we process proxied work at regular intervals.
+  // TODO(sbc): This should be remove 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);

test/codesize/test_codesize_minimal_pthreads.json

@@ -1,10 +1,10 @@
 {
   "a.out.js": 7884,
   "a.out.js.gz": 3854,
-  "a.out.nodebug.wasm": 19726,
-  "a.out.nodebug.wasm.gz": 9135,
-  "total": 27610,
-  "total_gz": 12989,
+  "a.out.nodebug.wasm": 19794,
+  "a.out.nodebug.wasm.gz": 9214,
+  "total": 27678,
+  "total_gz": 13068,
   "sent": [
     "a (memory)",
     "b (emscripten_get_now)",

test/codesize/test_codesize_minimal_pthreads_memgrowth.json

@@ -1,10 +1,10 @@
 {
   "a.out.js": 8306,
   "a.out.js.gz": 4059,
-  "a.out.nodebug.wasm": 19727,
-  "a.out.nodebug.wasm.gz": 9137,
-  "total": 28033,
-  "total_gz": 13196,
+  "a.out.nodebug.wasm": 19795,
+  "a.out.nodebug.wasm.gz": 9215,
+  "total": 28101,
+  "total_gz": 13274,
   "sent": [
     "a (memory)",
     "b (emscripten_get_now)",

tools/system_libs.py

@@ -67,7 +67,7 @@ def get_base_cflags(build_dir, force_object_files=False, preprocess=True):
     # caching of `pic` libraries (see `get_lib_dir` in `cache.py`)
     # FIXME(sbc): `-fPIC` should really be enough here.
     flags += ['-fPIC', '-sMAIN_MODULE']
-    if preprocess:
+    if not settings.WASM_WORKERS and preprocess:
       flags += ['-DEMSCRIPTEN_DYNAMIC_LINKING']
   if settings.MEMORY64:
     flags += ['-sMEMORY64']
@@ -735,7 +735,7 @@ def vary_on(cls):
   def get_default_variation(cls, **kwargs):
     return super().get_default_variation(
       is_mt=settings.PTHREADS,
-      is_ww=settings.SHARED_MEMORY and not settings.PTHREADS,
+      is_ww=settings.SHARED_MEMORY and not settings.PTHREADS and not settings.MAIN_MODULE,
       **kwargs,
     )
 
@@ -746,6 +746,9 @@ def variations(cls):
     # These are mutually exclusive, only one flag will be set at any give time.
     return [combo for combo in combos if not combo['is_mt'] or not combo['is_ww']]
 
+  def can_build(self):
+    return super().can_build() and not (settings.MAIN_MODULE and self.is_ww)
+
 
 class DebugLibrary(Library):
   def __init__(self, **kwargs):