[dylink] Fix deadlock in _emscripten_dlsync_threads()

system/lib/libc/dynlink.c

@@ -361,14 +361,14 @@ static void thread_sync_done(void* arg) {
 // `_emscripten_proxy_dlsync` below, and processed by background threads
 // that call `_emscripten_process_dlopen_queue` during futex_wait (i.e. whenever
 // they block).
-static em_proxying_queue * _Atomic dlopen_proxying_queue = NULL;
+em_proxying_queue* _Atomic _dlopen_proxying_queue = NULL;
 static thread_local bool processing_queue = false;
 
 void _emscripten_process_dlopen_queue() {
-  if (dlopen_proxying_queue && !processing_queue) {
+  if (_dlopen_proxying_queue && !processing_queue) {
     assert(!emscripten_is_main_runtime_thread());
     processing_queue = true;
-    emscripten_proxy_execute_queue(dlopen_proxying_queue);
+    emscripten_proxy_execute_queue(_dlopen_proxying_queue);
     processing_queue = false;
   }
 }
@@ -379,8 +379,8 @@ void _emscripten_process_dlopen_queue() {
 // manages the worker pool.
 int _emscripten_proxy_dlsync_async(pthread_t target_thread, em_promise_t promise) {
   assert(emscripten_is_main_runtime_thread());
-  if (!dlopen_proxying_queue) {
-    dlopen_proxying_queue = em_proxying_queue_create();
+  if (!_dlopen_proxying_queue) {
+    _dlopen_proxying_queue = em_proxying_queue_create();
   }
 
   struct promise_result* info = malloc(sizeof(struct promise_result));
@@ -391,7 +391,7 @@ int _emscripten_proxy_dlsync_async(pthread_t target_thread, em_promise_t promise
     .promise = promise,
     .result = false,
   };
-  int rtn = emscripten_proxy_callback(dlopen_proxying_queue,
+  int rtn = emscripten_proxy_callback(_dlopen_proxying_queue,
                                       target_thread,
                                       do_thread_sync,
                                       thread_sync_done,
@@ -403,21 +403,18 @@ int _emscripten_proxy_dlsync_async(pthread_t target_thread, em_promise_t promise
     emscripten_promise_resolve(promise, EM_PROMISE_FULFILL, NULL);
     emscripten_promise_destroy(promise);
     free(info);
-  } else {
-    // Wake up the target thread in case it's blocked in futex_wait
-    _emscripten_thread_notify(target_thread);
   }
   return rtn;
 }
 
 int _emscripten_proxy_dlsync(pthread_t target_thread) {
   assert(emscripten_is_main_runtime_thread());
-  if (!dlopen_proxying_queue) {
-    dlopen_proxying_queue = em_proxying_queue_create();
+  if (!_dlopen_proxying_queue) {
+    _dlopen_proxying_queue = em_proxying_queue_create();
   }
   int result;
   if (!emscripten_proxy_sync(
-        dlopen_proxying_queue, target_thread, do_thread_sync_out, &result)) {
+        _dlopen_proxying_queue, target_thread, do_thread_sync_out, &result)) {
     return 0;
   }
   return result;

system/lib/pthread/emscripten_yield.c

@@ -13,8 +13,8 @@ void _emscripten_thread_crashed() {
   // Notify the main thread that the calling thread has crashed. The will bring
   // down the whole program next time the main thread calls `_emscripten_yield`.
   crashed_thread_id = pthread_self();
-  // Force the main runtime thread to wake up in case it is waiting in
-  // `_emscripten_thread_notify`.
+  // Force the main runtime thread to wake up in case it is blocked in
+  // `emscripten_futex_wait`.
   _emscripten_thread_notify(emscripten_main_runtime_thread_id());
 }
 

system/lib/pthread/proxying.c

@@ -163,12 +163,19 @@ static bool do_proxy(em_proxying_queue* q, pthread_t target_thread, task t) {
   }
 
   bool ret = em_task_queue_send(tasks, t);
+
   // When proxying work to the main thread using the system queue we have a
   // special case in that we need to wake the target thread in case it is in
-  // `emscripten_futex_wait`.
-  if (ret && is_system_queue &&
-      pthread_equal(target_thread, emscripten_main_runtime_thread_id())) {
-    DBG("waking main runtime thread using _emscripten_thread_notify");
+  // `emscripten_futex_wait`. Additionally, the _dlopen_proxying_queue also
+  // requires a wakeup of the target thread after enqueuing work.
+  bool needs_notify =
+#ifdef EMSCRIPTEN_DYNAMIC_LINKING
+    q == _dlopen_proxying_queue ||
+#endif
+    (is_system_queue &&
+     pthread_equal(target_thread, emscripten_main_runtime_thread_id()));
+  if (ret && needs_notify) {
+    DBG("waking target thread using _emscripten_thread_notify");
     _emscripten_thread_notify(target_thread);
   }
   return ret;

system/lib/pthread/threading_internal.h

@@ -7,6 +7,8 @@
 
 #pragma once
 
+#include <emscripten/proxying.h>
+
 #include <inttypes.h>
 #include <pthread.h>
 #include <stdbool.h>
@@ -82,6 +84,7 @@ int _emscripten_thread_is_valid(pthread_t thread);
 void _emscripten_thread_exit_joinable(pthread_t thread);
 void _emscripten_thread_exit(void* result);
 void _emscripten_process_dlopen_queue(void);
+extern em_proxying_queue* _Atomic _dlopen_proxying_queue;
 
 #if !defined(__EMSCRIPTEN_PTHREADS__) || defined(NDEBUG)
 #define emscripten_set_current_thread_status(newStatus)

test/codesize/test_codesize_minimal_pthreads.json

@@ -1,10 +1,10 @@
 {
   "a.out.js": 7110,
   "a.out.js.gz": 3524,
-  "a.out.nodebug.wasm": 19070,
-  "a.out.nodebug.wasm.gz": 8801,
-  "total": 26180,
-  "total_gz": 12325,
+  "a.out.nodebug.wasm": 19069,
+  "a.out.nodebug.wasm.gz": 8800,
+  "total": 26179,
+  "total_gz": 12324,
   "sent": [
     "a (memory)",
     "b (exit)",

test/codesize/test_codesize_minimal_pthreads_memgrowth.json

@@ -1,10 +1,10 @@
 {
   "a.out.js": 7518,
   "a.out.js.gz": 3728,
-  "a.out.nodebug.wasm": 19071,
-  "a.out.nodebug.wasm.gz": 8802,
-  "total": 26589,
-  "total_gz": 12530,
+  "a.out.nodebug.wasm": 19070,
+  "a.out.nodebug.wasm.gz": 8801,
+  "total": 26588,
+  "total_gz": 12529,
   "sent": [
     "a (memory)",
     "b (exit)",

test/core/pthread/test_pthread_dlopen.c

@@ -1,11 +1,10 @@
 #include <assert.h>
-#include <stdbool.h>
-#include <stdatomic.h>
 #include <dlfcn.h>
 #include <pthread.h>
 #include <stdio.h>
 #include <unistd.h>
-#include <emscripten/threading.h>
+
+#define NUM_THREADS 8
 
 typedef int* (*sidey_data_type)();
 typedef int (*func_t)();
@@ -16,18 +15,15 @@ static sidey_func_type p_side_func_address;
 static int* expected_data_addr;
 static func_t expected_func_addr;
 
-static atomic_bool started = false;
-static atomic_bool ready = false;
+pthread_barrier_t started;
 
 static void* thread_main(void* arg) {
-  printf("in thread_main\n");
-  started = true;
+  int id = (int)(intptr_t)arg;
 
-  while (!ready) {
-    printf("yielding ..\n");
-    sched_yield();
-    usleep(1000*100);
-  }
+  printf("thread %d: in thread_main\n", id);
+
+  // Wait until all threads + main have reached the barrier
+  pthread_barrier_wait(&started);
 
   int* data_addr = p_side_data_address();
   assert(data_addr == expected_data_addr);
@@ -36,20 +32,21 @@ static void* thread_main(void* arg) {
   assert(expected_func_addr == func_addr);
   assert(func_addr() == 43);
 
-  printf("thread_main done\n");
+  printf("thread %d: thread_main done\n", id);
   return 0;
 }
 
 int main() {
   printf("in main\n");
 
-  // Start a thread before loading the shared library
-  pthread_t t;
-  int rc = pthread_create(&t, NULL, thread_main, NULL);
-  assert(rc == 0);
+  pthread_barrier_init(&started, NULL, NUM_THREADS + 1);
+  pthread_t threads[NUM_THREADS];
 
-  // Spin until the thread has started
-  while (!started) {}
+  // Start threads before loading the shared library
+  for (int i = 0; i < NUM_THREADS; ++i) {
+    int rc = pthread_create(&threads[i], NULL, thread_main, (void*)(intptr_t)i);
+    assert(rc == 0);
+  }
 
   printf("loading dylib\n");
   void* handle = dlopen("libside.so", RTLD_NOW|RTLD_GLOBAL);
@@ -71,25 +68,15 @@ int main() {
   printf("p_side_func_address -> %p\n", expected_func_addr);
   assert(expected_func_addr() == 43);
 
-  ready = true;
-
-  printf("joining\n");
-  rc = pthread_join(t, NULL);
-  assert(rc == 0);
-  printf("done join\n");
-
-  printf("starting second & third thread\n");
-  pthread_t t2, t3;
-  rc = pthread_create(&t2, NULL, thread_main, NULL);
-  assert(rc == 0);
-  rc = pthread_create(&t3, NULL, thread_main, NULL);
-  assert(rc == 0);
-  rc = pthread_join(t2, NULL);
-  assert(rc == 0);
-  rc = pthread_join(t3, NULL);
-  assert(rc == 0);
-  printf("starting second & third thread\n");
+  // Wait until all threads execute their entry points
+  pthread_barrier_wait(&started);
+  for (int i = 0; i < NUM_THREADS; ++i) {
+    int rc = pthread_join(threads[i], NULL);
+    assert(rc == 0);
+  }
 
   dlclose(handle);
+
+  printf("done\n");
   return 0;
 }

test/test_browser.py

@@ -3503,6 +3503,10 @@ def test_dlopen_blocking(self):
     # But with PROXY_TO_PTHEAD it does work, since we can do blocking and sync XHR in a worker.
     self.btest_exit('other/test_dlopen_blocking.c', cflags=['-sMAIN_MODULE=2', '-sPROXY_TO_PTHREAD', '-pthread', '-Wno-experimental', '-sAUTOLOAD_DYLIBS=0', 'libside.so'])
 
+  def test_pthread_dlopen(self):
+    self.emcc('core/pthread/test_pthread_dlopen_side.c', ['-o', 'libside.so', '-sSIDE_MODULE', '-pthread', '-Wno-experimental'])
+    self.btest_exit('core/pthread/test_pthread_dlopen.c', cflags=['-sMAIN_MODULE=2', '-sEXIT_RUNTIME', '-sPTHREAD_POOL_SIZE=8', '-pthread', '-Wno-experimental', 'libside.so'])
+
   # verify that dynamic linking works in all kinds of in-browser environments.
   # don't mix different kinds in a single test.
   @parameterized({

test/test_core.py

@@ -9302,18 +9302,21 @@ def test_pthread_dylink_exceptions(self):
     self.dylink_testf(test_file('core/pthread/test_pthread_dylink_exceptions.cpp'))
 
   @needs_dylink
+  @parameterized({
+    '': ([],),
+    'proxied': (['-sPROXY_TO_PTHREAD'],),
+  })
   @requires_pthreads
-  def test_pthread_dlopen(self):
+  def test_pthread_dlopen(self, args):
     self.cflags += ['-Wno-experimental', '-pthread']
     self.build_dlfcn_lib(test_file('core/pthread/test_pthread_dlopen_side.c'))
 
     self.cflags += ['--embed-file', 'libside.so@libside.so']
     self.prep_dlfcn_main()
     self.set_setting('EXIT_RUNTIME')
-    self.set_setting('PROXY_TO_PTHREAD')
     self.do_runf('core/pthread/test_pthread_dlopen.c',
-                 ['side module ctor', 'done join', 'side module atexit'],
-                 assert_all=True)
+                 ['side module ctor', 'done', 'side module atexit'],
+                 assert_all=True, cflags=args)
 
   @needs_dylink
   @requires_pthreads