Split out housekeeping tasks into new emscripten_yield function. NFC (#16521)

We currently only do two thing in this function:

1. Check if we should die because some other thread crashed.
2. Process the work queue.

However, I have plans to do other things such as keep loaded code
in sync across threads.

Author: sbc100

system/include/emscripten/threading.h

@@ -289,6 +289,8 @@ void emscripten_check_blocking_allowed(void);
 // Experimental API for syncing loaded code between pthreads.
 void _emscripten_thread_sync_code();
 
+void _emscripten_yield();
+
 #ifdef __cplusplus
 }
 #endif

system/lib/libc/musl/src/sched/sched_yield.c

@@ -9,9 +9,9 @@ int sched_yield()
 {
 #if __EMSCRIPTEN__
 	// SharedArrayBuffer and wasm threads do not support explicit yielding.
-	// For now we at least process our event queue so that other threads who
-	// are waiting on this one to perform actions can make progesss.
-	emscripten_current_thread_process_queued_calls();
+	// For now we at least call `emscripten_yield` which processes the event queue
+	// (along with other essential tasks).
+	_emscripten_yield();
 	return 0;
 #else
 	return syscall(SYS_sched_yield);

system/lib/pthread/emscripten_futex_wait.c

@@ -8,7 +8,6 @@
 #include <errno.h>
 #include <math.h>
 #include <emscripten/threading.h>
-#include <stdbool.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include "atomic.h"
@@ -28,13 +27,13 @@ static int futex_wait_busy(volatile void *addr, uint32_t val, double timeout) {
   // 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.
+  // then call _emscripten_yield() 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.
   void* last_addr = a_cas_p(&_emscripten_main_thread_futex, 0, (void*)addr);
   // We must not have already been waiting.
   assert(last_addr == 0);
@@ -62,13 +61,13 @@ static int futex_wait_busy(volatile void *addr, uint32_t val, double timeout) {
       // We were told to stop waiting, so stop.
       break;
     }
-    emscripten_main_thread_process_queued_calls();
+    _emscripten_yield();
 
     // 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(),
+    //  * recurse into _emscripten_yield(),
     //    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
@@ -83,7 +82,7 @@ static int futex_wait_busy(volatile void *addr, uint32_t val, double timeout) {
     // 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
+    // _emscripten_yield(), 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
@@ -92,11 +91,11 @@ static int futex_wait_busy(volatile void *addr, uint32_t val, double timeout) {
     // 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
+    // _emscripten_yield(). 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()
+    // was the same as calling _emscripten_yield()
     // a few times before calling emscripten_futex_wait().
     if (__c11_atomic_load((_Atomic uintptr_t*)addr, __ATOMIC_SEQ_CST) != val) {
       return -EWOULDBLOCK;
@@ -109,32 +108,12 @@ static int futex_wait_busy(volatile void *addr, uint32_t val, double timeout) {
   return 0;
 }
 
-static _Atomic bool thread_crashed = false;
-
-void _emscripten_thread_crashed() {
-  thread_crashed = true;
-}
-
 int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms) {
   if ((((intptr_t)addr)&3) != 0) {
     return -EINVAL;
   }
 
-  if (emscripten_is_main_runtime_thread()) {
-    // When a secondary thread crashes, we need to be able to interrupt the main
-    // thread even if it's in a blocking/looping on a mutex.  We want to avoid
-    // using the normal proxying mechanism to send this message since it can
-    // allocate (or otherwise itself crash) so use a low level atomic primitive
-    // for this signal.
-    if (thread_crashed) {
-      // Return the event loop so we can handle the message from the crashed
-      // thread.
-      emscripten_unwind_to_js_event_loop();
-    }
-
-    // Assist other threads by executing proxied operations that are effectively singlethreaded.
-    emscripten_main_thread_process_queued_calls();
-  }
+  _emscripten_yield();
 
   int ret;
   emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_WAITFUTEX);

system/lib/pthread/emscripten_yield.c

@@ -0,0 +1,41 @@
+#include <stdbool.h>
+#include <sched.h>
+#include <threads.h>
+#include "syscall.h"
+
+#include <emscripten/threading.h>
+
+static _Atomic bool thread_crashed = false;
+
+void _emscripten_thread_crashed() {
+  thread_crashed = true;
+}
+
+/*
+ * Called whenever a thread performs a blocking action (or calls sched_yield).
+ * This function takes care of running the event queue and other housekeeping
+ * tasks.
+ */
+void _emscripten_yield() {
+  int is_runtime_thread = emscripten_is_main_runtime_thread();
+
+  // When a secondary thread crashes, we need to be able to interrupt the main
+  // thread even if it's in a blocking/looping on a mutex.  We want to avoid
+  // using the normal proxying mechanism to send this message since it can
+  // allocate (or otherwise itself crash) so use a low level atomic primitive
+  // for this signal.
+  if (is_runtime_thread) {
+    if (thread_crashed) {
+      // Return the event loop so we can handle the message from the crashed
+      // thread.
+      emscripten_unwind_to_js_event_loop();
+    }
+
+    // Assist other threads by executing proxied operations that are effectively
+    // singlethreaded.
+    emscripten_main_thread_process_queued_calls();
+  }
+
+  // TODO(sbc): Do code synchronization between threads as part of yield
+  //_emscripten_thread_sync_code();
+}

system/lib/pthread/library_pthread_stub.c

@@ -46,6 +46,10 @@ void emscripten_current_thread_process_queued_calls() {
   // nop
 }
 
+void _emscripten_yield() {
+  // nop
+}
+
 int pthread_mutex_init(
   pthread_mutex_t* __restrict mutex, const pthread_mutexattr_t* __restrict attr) {
   return 0;

tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.funcs

@@ -19,6 +19,7 @@ $_emscripten_thread_crashed
 $_emscripten_thread_exit
 $_emscripten_thread_free_data
 $_emscripten_thread_init
+$_emscripten_yield
 $_main_thread
 $a_cas
 $a_cas_p.1

tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size

@@ -1 +1 @@
-18576
+18582

tools/system_libs.py

@@ -852,6 +852,7 @@ def get_files(self):
           'emscripten_thread_state.S',
           'emscripten_futex_wait.c',
           'emscripten_futex_wake.c',
+          'emscripten_yield.c',
         ])
     else:
       ignore += ['thread']