Add new emscripten_thread_can_block function and use it in emscripten_futex_wait. NFC

sbc100 · sbc100 · commit a30cc1591ab3 · 2021-12-12T10:43:05.000-08:00
This new function returns true only on the main browser thread and not
on the main node runtime thread (unlike
`emscripten_is_main_browser_thread` which is an historical misnomer).

Using this new native function along with the intrisic
`__builtin_wasm_memory_atomic_wait32` we can now implement
`emscripten_futex_wait` in native code and only call out to JS for the
non-block/busy-looping case.

As a followup we could also move the non-block/busy-looping code
to the native side but doing it this way keep this change small
and focused.
diff --git a/src/library_pthread.js b/src/library_pthread.js
@@ -598,7 +598,7 @@ var LibraryPThread = {
     // Pass the thread address to the native code where they stored in wasm
     // globals which act as a form of TLS. Global constructors trying
     // to access this value will read the wrong value, but that is UB anyway.
-    __emscripten_thread_init(tb, /*isMainBrowserThread=*/!ENVIRONMENT_IS_WORKER, /*isMainRuntimeThread=*/1);
+    __emscripten_thread_init(tb, /*isMainBrowserThread=*/!ENVIRONMENT_IS_WORKER, /*isMainRuntimeThread=*/1, /*canBlock=*/!ENVIRONMENT_IS_WEB);
 #if ASSERTIONS
     PThread.mainRuntimeThread = true;
     // Verify that this native symbol used by futex_wait/wake is exported correctly.
@@ -843,134 +843,112 @@ var LibraryPThread = {
   },
 
   // Returns 0 on success, or one of the values -ETIMEDOUT, -EWOULDBLOCK or -EINVAL on error.
-  emscripten_futex_wait__deps: ['emscripten_main_thread_process_queued_calls'],
-  emscripten_futex_wait: function(addr, val, timeout) {
-    if (addr <= 0 || addr > HEAP8.length || addr&3 != 0) return -{{{ cDefine('EINVAL') }}};
-    // We can do a normal blocking wait anywhere but on the main browser thread.
-    if (!ENVIRONMENT_IS_WEB) {
-#if PTHREADS_PROFILING
-      PThread.setThreadStatusConditional(_pthread_self(), {{{ cDefine('EM_THREAD_STATUS_RUNNING') }}}, {{{ cDefine('EM_THREAD_STATUS_WAITFUTEX') }}});
-#endif
-      var ret = Atomics.wait(HEAP32, addr >> 2, val, timeout);
-#if PTHREADS_PROFILING
-      PThread.setThreadStatusConditional(_pthread_self(), {{{ cDefine('EM_THREAD_STATUS_WAITFUTEX') }}}, {{{ cDefine('EM_THREAD_STATUS_RUNNING') }}});
-#endif
-      if (ret === 'timed-out') return -{{{ cDefine('ETIMEDOUT') }}};
-      if (ret === 'not-equal') return -{{{ cDefine('EWOULDBLOCK') }}};
-      if (ret === 'ok') return 0;
-      throw 'Atomics.wait returned an unexpected value ' + ret;
-    } else {
-      // First, check if the value is correct for us to wait on.
-      if (Atomics.load(HEAP32, addr >> 2) != val) {
-        return -{{{ cDefine('EWOULDBLOCK') }}};
-      }
-
-      // Atomics.wait is not available in the main browser thread, so simulate it via busy spinning.
-      var tNow = performance.now();
-      var tEnd = tNow + timeout;
-
-#if PTHREADS_PROFILING
-      PThread.setThreadStatusConditional(_pthread_self(), {{{ cDefine('EM_THREAD_STATUS_RUNNING') }}}, {{{ cDefine('EM_THREAD_STATUS_WAITFUTEX') }}});
-#endif
-      // Register globally which address the main thread is simulating to be
-      // waiting on. When zero, the main thread is not waiting on anything, and on
-      // nonzero, the contents of the address pointed by __emscripten_main_thread_futex
-      // tell which address the main thread is simulating its wait on.
-      // We need to be careful of recursion here: If we wait on a futex, and
-      // then call _emscripten_main_thread_process_queued_calls() below, that
-      // will call code that takes the proxying mutex - which can once more
-      // reach this code in a nested call. To avoid interference between the
-      // two (there is just a single __emscripten_main_thread_futex at a time), unmark
-      // ourselves before calling the potentially-recursive call. See below for
-      // how we handle the case of our futex being notified during the time in
-      // between when we are not set as the value of __emscripten_main_thread_futex.
-      var lastAddr = Atomics.exchange(HEAP32, __emscripten_main_thread_futex >> 2, addr);
+  _emscripten_futex_wait_non_blocking__deps: ['emscripten_main_thread_process_queued_calls'],
+  _emscripten_futex_wait_non_blocking: function(addr, val, timeout) {
 #if ASSERTIONS
-      // We must not have already been waiting.
-      assert(lastAddr == 0);
-#endif
-
-      while (1) {
-        // Check for a timeout.
-        tNow = performance.now();
-        if (tNow > tEnd) {
-#if PTHREADS_PROFILING
-          PThread.setThreadStatusConditional(_pthread_self(), {{{ cDefine('EM_THREAD_STATUS_RUNNING') }}}, {{{ cDefine('EM_THREAD_STATUS_WAITFUTEX') }}});
+    // Should only be called from the main web thread where atomics.wait is not allowed.
+    assert(ENVIRONMENT_IS_WEB);
+#endif
+
+    // Atomics.wait is not available in the main browser thread, so simulate it via busy spinning.
+    var tNow = performance.now();
+    var tEnd = tNow + timeout;
+
+    // Register globally which address the main thread is simulating to be
+    // waiting on. When zero, the main thread is not waiting on anything, and on
+    // nonzero, the contents of the address pointed by __emscripten_main_thread_futex
+    // tell which address the main thread is simulating its wait on.
+    // We need to be careful of recursion here: If we wait on a futex, and
+    // then call _emscripten_main_thread_process_queued_calls() below, that
+    // will call code that takes the proxying mutex - which can once more
+    // reach this code in a nested call. To avoid interference between the
+    // two (there is just a single __emscripten_main_thread_futex at a time), unmark
+    // ourselves before calling the potentially-recursive call. See below for
+    // how we handle the case of our futex being notified during the time in
+    // between when we are not set as the value of __emscripten_main_thread_futex.
+#if ASSERTIONS
+    assert(__emscripten_main_thread_futex > 0);
 #endif
-          // We timed out, so stop marking ourselves as waiting.
-          lastAddr = Atomics.exchange(HEAP32, __emscripten_main_thread_futex >> 2, 0);
+    var lastAddr = Atomics.exchange(HEAP32, __emscripten_main_thread_futex >> 2, addr);
 #if ASSERTIONS
-          // The current value must have been our address which we set, or
-          // in a race it was set to 0 which means another thread just allowed
-          // us to run, but (tragically) that happened just a bit too late.
-          assert(lastAddr == addr || lastAddr == 0);
+    // We must not have already been waiting.
+    assert(lastAddr == 0);
 #endif
-          return -{{{ cDefine('ETIMEDOUT') }}};
-        }
-        // We are performing a blocking loop here, so we must handle proxied
-        // events from pthreads, to avoid deadlocks.
-        // Note that we have to do so carefully, as we may take a lock while
-        // doing so, which can recurse into this function; stop marking
-        // ourselves as waiting while we do so.
+
+    while (1) {
+      // Check for a timeout.
+      tNow = performance.now();
+      if (tNow > tEnd) {
+        // We timed out, so stop marking ourselves as waiting.
         lastAddr = Atomics.exchange(HEAP32, __emscripten_main_thread_futex >> 2, 0);
 #if ASSERTIONS
+        // The current value must have been our address which we set, or
+        // in a race it was set to 0 which means another thread just allowed
+        // us to run, but (tragically) that happened just a bit too late.
         assert(lastAddr == addr || lastAddr == 0);
 #endif
-        if (lastAddr == 0) {
-          // We were told to stop waiting, so stop.
-          break;
-        }
-        _emscripten_main_thread_process_queued_calls();
-
-        // Check the value, as if we were starting the futex all over again.
-        // This handles the following case:
-        //
-        //  * wait on futex A
-        //  * recurse into emscripten_main_thread_process_queued_calls(),
-        //    which waits on futex B. that sets the __emscripten_main_thread_futex address to
-        //    futex B, and there is no longer any mention of futex A.
-        //  * a worker is done with futex A. it checks __emscripten_main_thread_futex but does
-        //    not see A, so it does nothing special for the main thread.
-        //  * a worker is done with futex B. it flips mainThreadMutex from B
-        //    to 0, ending the wait on futex B.
-        //  * we return to the wait on futex A. __emscripten_main_thread_futex is 0, but that
-        //    is because of futex B being done - we can't tell from
-        //    __emscripten_main_thread_futex whether A is done or not. therefore, check the
-        //    memory value of the futex.
-        //
-        // That case motivates the design here. Given that, checking the memory
-        // address is also necessary for other reasons: we unset and re-set our
-        // address in __emscripten_main_thread_futex around calls to
-        // emscripten_main_thread_process_queued_calls(), and a worker could
-        // attempt to wake us up right before/after such times.
-        //
-        // Note that checking the memory value of the futex is valid to do: we
-        // could easily have been delayed (relative to the worker holding on
-        // to futex A), which means we could be starting all of our work at the
-        // later time when there is no need to block. The only "odd" thing is
-        // that we may have caused side effects in that "delay" time. But the
-        // only side effects we can have are to call
-        // emscripten_main_thread_process_queued_calls(). That is always ok to
-        // do on the main thread (it's why it is ok for us to call it in the
-        // middle of this function, and elsewhere). So if we check the value
-        // here and return, it's the same is if what happened on the main thread
-        // was the same as calling emscripten_main_thread_process_queued_calls()
-        // a few times times before calling emscripten_futex_wait().
-        if (Atomics.load(HEAP32, addr >> 2) != val) {
-          return -{{{ cDefine('EWOULDBLOCK') }}};
-        }
-
-        // Mark us as waiting once more, and continue the loop.
-        lastAddr = Atomics.exchange(HEAP32, __emscripten_main_thread_futex >> 2, addr);
+        return -{{{ cDefine('ETIMEDOUT') }}};
+      }
+      // We are performing a blocking loop here, so we must handle proxied
+      // events from pthreads, to avoid deadlocks.
+      // Note that we have to do so carefully, as we may take a lock while
+      // doing so, which can recurse into this function; stop marking
+      // ourselves as waiting while we do so.
+      lastAddr = Atomics.exchange(HEAP32, __emscripten_main_thread_futex >> 2, 0);
 #if ASSERTIONS
-        assert(lastAddr == 0);
+      assert(lastAddr == addr || lastAddr == 0);
 #endif
+      if (lastAddr == 0) {
+        // We were told to stop waiting, so stop.
+        break;
       }
-#if PTHREADS_PROFILING
-      PThread.setThreadStatusConditional(_pthread_self(), {{{ cDefine('EM_THREAD_STATUS_RUNNING') }}}, {{{ cDefine('EM_THREAD_STATUS_WAITFUTEX') }}});
+      _emscripten_main_thread_process_queued_calls();
+
+      // Check the value, as if we were starting the futex all over again.
+      // This handles the following case:
+      //
+      //  * wait on futex A
+      //  * recurse into emscripten_main_thread_process_queued_calls(),
+      //    which waits on futex B. that sets the __emscripten_main_thread_futex address to
+      //    futex B, and there is no longer any mention of futex A.
+      //  * a worker is done with futex A. it checks __emscripten_main_thread_futex but does
+      //    not see A, so it does nothing special for the main thread.
+      //  * a worker is done with futex B. it flips mainThreadMutex from B
+      //    to 0, ending the wait on futex B.
+      //  * we return to the wait on futex A. __emscripten_main_thread_futex is 0, but that
+      //    is because of futex B being done - we can't tell from
+      //    __emscripten_main_thread_futex whether A is done or not. therefore, check the
+      //    memory value of the futex.
+      //
+      // That case motivates the design here. Given that, checking the memory
+      // address is also necessary for other reasons: we unset and re-set our
+      // address in __emscripten_main_thread_futex around calls to
+      // emscripten_main_thread_process_queued_calls(), and a worker could
+      // attempt to wake us up right before/after such times.
+      //
+      // Note that checking the memory value of the futex is valid to do: we
+      // could easily have been delayed (relative to the worker holding on
+      // to futex A), which means we could be starting all of our work at the
+      // later time when there is no need to block. The only "odd" thing is
+      // that we may have caused side effects in that "delay" time. But the
+      // only side effects we can have are to call
+      // emscripten_main_thread_process_queued_calls(). That is always ok to
+      // do on the main thread (it's why it is ok for us to call it in the
+      // middle of this function, and elsewhere). So if we check the value
+      // here and return, it's the same is if what happened on the main thread
+      // was the same as calling emscripten_main_thread_process_queued_calls()
+      // a few times times before calling emscripten_futex_wait().
+      if (Atomics.load(HEAP32, addr >> 2) != val) {
+        return -{{{ cDefine('EWOULDBLOCK') }}};
+      }
+
+      // Mark us as waiting once more, and continue the loop.
+      lastAddr = Atomics.exchange(HEAP32, __emscripten_main_thread_futex >> 2, addr);
+#if ASSERTIONS
+      assert(lastAddr == 0);
 #endif
-      return 0;
     }
+    return 0;
   },
 
   // Returns the number of threads (>= 0) woken up, or the value -EINVAL on error.
diff --git a/src/worker.js b/src/worker.js
@@ -188,7 +188,7 @@ self.onmessage = function(e) {
       Module['__performance_now_clock_drift'] = performance.now() - e.data.time;
 
       // Pass the thread address inside the asm.js scope to store it for fast access that avoids the need for a FFI out.
-      Module['__emscripten_thread_init'](e.data.threadInfoStruct, /*isMainBrowserThread=*/0, /*isMainRuntimeThread=*/0);
+      Module['__emscripten_thread_init'](e.data.threadInfoStruct, /*isMainBrowserThread=*/0, /*isMainRuntimeThread=*/0, /*canBlock=*/1);
 
 #if ASSERTIONS
       assert(e.data.threadInfoStruct);
diff --git a/system/include/emscripten/threading.h b/system/include/emscripten/threading.h
@@ -296,9 +296,17 @@ int emscripten_dispatch_to_thread_async_(pthread_t target_thread,
 // Returns 1 if the current thread is the thread that hosts the Emscripten runtime.
 int emscripten_is_main_runtime_thread(void);
 
-// Returns 1 if the current thread is the main browser thread.
+// Returns 1 if the current thread is the main browser thread.  For historical
+// reasons this functions also returns 1 on the main node thread.
+// Use emscripten_thread_can_block() instead to determine if the current
+// thread can block.
 int emscripten_is_main_browser_thread(void);
 
+// Returns 1 if the current thread can use atomics.wait or other forms of
+// synchronous blocking.  This will return 0 on the main web browser thread
+// and 1 everywhere else, including on the main node thread.
+int emscripten_thread_can_block(void);
+
 // A temporary workaround to issue
 // https://github.com/emscripten-core/emscripten/issues/3495:
 // Call this in the body of all lock-free atomic (cas) loops that the main
diff --git a/system/lib/pthread/emscripten_futex_wait.c b/system/lib/pthread/emscripten_futex_wait.c
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2021 The Emscripten Authors.  All rights reserved.
+ * Emscripten is available under two separate licenses, the MIT license and the
+ * University of Illinois/NCSA Open Source License.  Both these licenses can be
+ * found in the LICENSE file.
+ */
+#include <assert.h>
+#include <errno.h>
+#include <math.h>
+#include <emscripten/threading.h>
+
+int _emscripten_futex_wait_non_blocking(volatile void *addr, uint32_t val, double max_wait_ms);
+
+int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms) {
+  if ((((intptr_t)addr)&3) != 0) {
+    return -EINVAL;
+  }
+
+  int ret;
+  emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_WAITFUTEX);
+
+  // For threads that cannot block (i.e. the main browser thread) we can't use
+  // __builtin_wasm_memory_atomic_wait32 so we call out the JS function that
+  // will busy wait.
+  if (!emscripten_thread_can_block()) {
+    ret = _emscripten_futex_wait_non_blocking(addr, val, max_wait_ms);
+    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_ms = (int64_t)(max_wait_ms*1000*1000);
+  }
+  ret = __builtin_wasm_memory_atomic_wait32((int*)addr, val, max_wait_ns);
+  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) {
+    return -EWOULDBLOCK;
+  }
+  if (ret == 2) {
+    return -ETIMEDOUT;
+  }
+  assert(ret == 0);
+  return 0;
+}
diff --git a/system/lib/pthread/emscripten_thread_state.S b/system/lib/pthread/emscripten_thread_state.S
@@ -5,8 +5,8 @@
 #endif
 
 .globaltype __tls_base, PTR
-.globaltype thread_ptr, PTR
 
+.globaltype thread_ptr, PTR
 thread_ptr:
 
 .globaltype is_main_thread, i32
@@ -15,6 +15,9 @@ is_main_thread:
 .globaltype is_runtime_thread, i32
 is_runtime_thread:
 
+.globaltype can_block, i32
+can_block:
+
 .globl __get_tp
 __get_tp:
   .functype __get_tp () -> (PTR)
@@ -23,13 +26,15 @@ __get_tp:
 
 .globl _emscripten_thread_init
 _emscripten_thread_init:
-  .functype _emscripten_thread_init (PTR, i32, i32) -> ()
+  .functype _emscripten_thread_init (PTR, i32, i32, i32) -> ()
   local.get 0
   global.set thread_ptr
   local.get 1
   global.set is_main_thread
   local.get 2
   global.set is_runtime_thread
+  local.get 3
+  global.set can_block
   end_function
 
 # Accessor for `__tls_base` symbol which is a wasm global an not directly
@@ -53,3 +58,10 @@ emscripten_is_main_browser_thread:
   .functype emscripten_is_main_browser_thread () -> (i32)
   global.get is_main_thread
   end_function
+
+# Semantically the same as testing "!ENVIRONMENT_IS_WEB" in JS
+.globl emscripten_thread_can_block
+emscripten_thread_can_block:
+  .functype emscripten_thread_can_block () -> (i32)
+  global.get can_block
+  end_function
diff --git a/system/lib/pthread/pthread_create.c b/system/lib/pthread/pthread_create.c
@@ -23,7 +23,7 @@
 // See musl's pthread_create.c
 
 extern int __pthread_create_js(struct pthread *thread, const pthread_attr_t *attr, void *(*start_routine) (void *), void *arg);
-extern void _emscripten_thread_init(int, int, int);
+extern void _emscripten_thread_init(int, int, int, int);
 extern int _emscripten_default_pthread_stack_size();
 extern void __pthread_detached_exit();
 extern void* _emscripten_tls_base();
@@ -225,7 +225,7 @@ void _emscripten_thread_exit(void* result) {
   self->tsd = NULL;
 
   // Not hosting a pthread anymore in this worker set __pthread_self to NULL
-  _emscripten_thread_init(0, 0, 0);
+  _emscripten_thread_init(0, 0, 0, 1);
 
   /* This atomic potentially competes with a concurrent pthread_detach
    * call; the loser is responsible for freeing thread resources. */
diff --git a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.funcs b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.funcs
@@ -32,6 +32,7 @@ $em_queued_call_malloc
 $emscripten_async_run_in_main_thread
 $emscripten_current_thread_process_queued_calls
 $emscripten_dispatch_to_thread_
+$emscripten_futex_wait
 $emscripten_main_thread_process_queued_calls
 $emscripten_proxy_main
 $emscripten_run_in_main_runtime_thread_js
diff --git a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size
@@ -1 +1 @@
-16699
+16817
diff --git a/tools/system_libs.py b/tools/system_libs.py
