Block in dlopen until all threads have loaded the module (#18376)

Fixes: #18345

Author: sbc100

emcc.py

@@ -1653,7 +1653,13 @@ def setup_pthreads(target):
   ]
 
   if settings.MAIN_MODULE:
-    settings.REQUIRED_EXPORTS += ['_emscripten_thread_sync_code', '__dl_seterr']
+    settings.REQUIRED_EXPORTS += [
+      '_emscripten_dlsync_self',
+      '_emscripten_dlsync_self_async',
+      '_emscripten_proxy_dlsync',
+      '_emscripten_proxy_dlsync_async',
+      '__dl_seterr',
+    ]
 
   settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += [
     '$exitOnMainThread',

site/source/docs/compiling/Dynamic-Linking.rst

@@ -206,12 +206,11 @@ the stack trace (in an unminified build); building with
 Limitations
 -----------
 
--  Chromium does not support compiling >4kB WASM on the main thread, and
-   that includes side modules; you can use ``--use-preload-plugins`` (in
-   ``emcc`` or ``file_packager.py``) to make Emscripten compile them on
-   startup
-   `[doc] <https://emscripten.org/docs/porting/files/packaging_files.html#preloading-files>`__
-   `[discuss] <https://groups.google.com/forum/#!topic/emscripten-discuss/cE3hUV3fDSw>`__.
+- Chromium does not support compiling >4kB WASM on the main thread, and that
+  includes side modules; you can use ``--use-preload-plugins`` (in ``emcc`` or
+  ``file_packager.py``) to make Emscripten compile them on startup
+  `[doc] <https://emscripten.org/docs/porting/files/packaging_files.html#preloading-files>`__
+  `[discuss] <https://groups.google.com/forum/#!topic/emscripten-discuss/cE3hUV3fDSw>`__.
 - ``EM_ASM`` code defined within side modules depends on ``eval`` support are
   is therefore incompatible with ``-sDYNAMIC_EXECUTION=0``.
 - ``EM_JS`` functions defined in side modules are not yet supported.
@@ -220,10 +219,26 @@ Limitations
 Pthreads support
 ----------------
 
-Dynamic linking + pthreads is is still experimental.  While you can link with
-``MAIN_MODULE`` and ``-pthread`` emscripten will produce a warning by default
-when you do this.
-
-While load-time dynamic linking should largely work and does not have any major
-known issues, runtime dynamic linking (with ``dlopen()``) has limited support
-when used with pthreads.
+Dynamic linking + pthreads is is still experimental.  As such, linking with both
+``MAIN_MODULE`` and ``-pthread`` will produce a warning.
+
+While load-time dynamic linking works without any complications, runtime dynamic
+linking via ``dlopen``/``dlsym`` can require some extra consideration.  The
+reason for this is that keeping the indirection function pointer table in sync
+between threads has to be done by emscripten library code.  Each time a new
+library is loaded or a new symbol is requested via ``dlsym``, table slots can be
+added and these changes need to be mirrored on every thread in the process.
+
+Changes to the table are protected by a mutex, and before any thread returns
+from ``dlopen`` or ``dlsym`` it will wait until all other threads are sync.  In
+order to make this synchronization as seamless as possible, we hook into the
+low level primitives of `emscripten_futex_wait` and `emscirpten_yield`.
+
+For most use cases all this happens under hood and no special action is needed.
+However, there there is one class of application that currently may require
+modification.  If your applications busy waits, or directly uses the
+``atomic.waitXX`` instructions (or the clang
+``__builtin_wasm_memory_atomic_waitXX`` builtins) you maybe need to switch it
+to use ``emscripten_futex_wait`` or order avoid deadlocks.  If you don't use
+``emscripten_futex_wait`` while you block, you could potentially block other
+threads that are calling ``dlopen`` and/or ``dlsym``.

src/library_promise.js

@@ -12,19 +12,25 @@ mergeInto(LibraryManager.library, {
   $getPromise: function(id) {
     return promiseMap.get(id).promise;
   },
-  emscripten_promise_create__deps: ['$promiseMap'],
-  emscripten_promise_create__sig: 'p',
-  emscripten_promise_create: function() {
+
+  $makePromise__deps: ['$promiseMap'],
+  $makePromise: function() {
     var promiseInfo = {};
     promiseInfo.promise = new Promise((resolve, reject) => {
       promiseInfo.reject = reject;
       promiseInfo.resolve = resolve;
     });
-    var id = promiseMap.allocate(promiseInfo);
+    promiseInfo.id = promiseMap.allocate(promiseInfo);
 #if RUNTIME_DEBUG
-    dbg('emscripten_promise_create: ' + id);
+    dbg('makePromise: ' + promiseInfo.id);
 #endif
-    return id;
+    return promiseInfo;
+  },
+
+  emscripten_promise_create__deps: ['$makePromise'],
+  emscripten_promise_create__sig: 'p',
+  emscripten_promise_create: function() {
+    return makePromise().id;
   },
 
   emscripten_promise_destroy__deps: ['$promiseMap'],
@@ -42,7 +48,7 @@ mergeInto(LibraryManager.library, {
   emscripten_promise_resolve__sig: 'vpip',
   emscripten_promise_resolve: function(id, result, value) {
 #if RUNTIME_DEBUG
-    err('emscripten_promise_resolve: ' + id);
+    dbg('emscripten_promise_resolve: ' + id);
 #endif
     var info = promiseMap.get(id);
     switch (result) {

src/library_pthread.js

@@ -31,6 +31,9 @@ var LibraryPThread = {
   $PThread__deps: ['_emscripten_thread_init',
                    '$killThread',
                    '$cancelThread', '$cleanupThread', '$zeroMemory',
+#if MAIN_MODULE
+                   '$markAsFinshed',
+#endif
                    '$spawnThread',
                    '_emscripten_thread_free_data',
                    'exit',
@@ -97,6 +100,12 @@ var LibraryPThread = {
       while (pthreadPoolSize--) {
         PThread.allocateUnusedWorker();
       }
+#endif
+#if MAIN_MODULE
+      PThread.outstandingPromises = {};
+      // Finished threads are threads that have finished running but we not yet
+      // joined.
+      PThread.finishedThreads = new Set();
 #endif
     },
 
@@ -269,6 +278,10 @@ var LibraryPThread = {
           spawnThread(d);
         } else if (cmd === 'cleanupThread') {
           cleanupThread(d['thread']);
+#if MAIN_MODULE
+        } else if (cmd === 'markAsFinshed') {
+          markAsFinshed(d['thread']);
+#endif
         } else if (cmd === 'killThread') {
           killThread(d['thread']);
         } else if (cmd === 'cancelThread') {
@@ -540,11 +553,20 @@ var LibraryPThread = {
   },
 
   $cleanupThread: function(pthread_ptr) {
+#if PTHREADS_DEBUG
+    dbg('cleanupThread: ' + ptrToString(pthread_ptr))
+#endif
 #if ASSERTIONS
     assert(!ENVIRONMENT_IS_PTHREAD, 'Internal Error! cleanupThread() can only ever be called from main application thread!');
     assert(pthread_ptr, 'Internal Error! Null pthread_ptr in cleanupThread!');
 #endif
     var worker = PThread.pthreads[pthread_ptr];
+#if MAIN_MODULE
+    PThread.finishedThreads.delete(pthread_ptr);
+    if (pthread_ptr in PThread.outstandingPromises) {
+      PThread.outstandingPromises[pthread_ptr].resolve();
+    }
+#endif
     assert(worker);
     PThread.returnWorkerToPool(worker);
   },
@@ -1048,7 +1070,7 @@ var LibraryPThread = {
     // Before we call the thread entry point, make sure any shared libraries
     // have been loaded on this there.  Otherwise our table migth be not be
     // in sync and might not contain the function pointer `ptr` at all.
-    __emscripten_thread_sync_code();
+    __emscripten_dlsync_self();
 #endif
     // pthread entry points are always of signature 'void *ThreadMain(void *arg)'
     // Native codebases sometimes spawn threads with other thread entry point
@@ -1077,6 +1099,99 @@ var LibraryPThread = {
 #endif
   },
 
+#if MAIN_MODULE
+  _emscripten_thread_exit_joinable: function(thread) {
+    // Called when a thread exits and is joinable.  We mark these threads
+    // as finished, which means that are in state where are no longer actually
+    // runnning, but remain around waiting to be joined.  In this state they
+    // cannot run any more proxied work.
+    if (!ENVIRONMENT_IS_PTHREAD) markAsFinshed(thread);
+    else postMessage({ 'cmd': 'markAsFinshed', 'thread': thread });
+  },
+
+  $markAsFinshed: function(pthread_ptr) {
+#if PTHREADS_DEBUG
+    dbg('markAsFinshed: ' + ptrToString(pthread_ptr));
+#endif
+    PThread.finishedThreads.add(pthread_ptr);
+    if (pthread_ptr in PThread.outstandingPromises) {
+      PThread.outstandingPromises[pthread_ptr].resolve();
+    }
+  },
+
+  // Asynchronous version dlsync_threads. Always run on the main thread.
+  // This work happens asynchronously. The `callback` is called once this work
+  // is completed, passing the ctx.
+  // TODO(sbc): Should we make a new form of __proxy attribute for JS library
+  // function that run asynchronously like but blocks the caller until they are
+  // done.  Perhaps "sync_with_ctx"?
+  _emscripten_dlsync_threads_async__sig: 'vppp',
+  _emscripten_dlsync_threads_async__deps: ['_emscripten_proxy_dlsync_async', 'emscripten_promise_create', '$getPromise'],
+  _emscripten_dlsync_threads_async: function(caller, callback, ctx) {
+#if PTHREADS_DEBUG
+    dbg("_emscripten_dlsync_threads_async caller=" + ptrToString(caller));
+#endif
+#if ASSERTIONS
+    assert(!ENVIRONMENT_IS_PTHREAD, 'Internal Error! _emscripten_dlsync_threads_async() can only ever be called from main thread');
+#endif
+
+    const promises = [];
+    assert(Object.keys(PThread.outstandingPromises).length === 0);
+
+    // This first promise resolves once the main thread has loaded all modules.
+    var info = makePromise();
+    promises.push(info.promise);
+    __emscripten_dlsync_self_async(info.id);
+
+
+    // We then create a sequence of promises, one per thread, that resolve once
+    // each thread has performed its sync using _emscripten_proxy_dlsync.
+    // Any new threads that are created after this call will automaticaly be
+    // in sync because we call `__emscripten_dlsync_self` in
+    // invokeEntryPoint before the threads entry point is called.
+    for (const ptr of Object.keys(PThread.pthreads)) {
+      const pthread_ptr = Number(ptr);
+      if (pthread_ptr !== caller && !PThread.finishedThreads.has(pthread_ptr)) {
+        info = makePromise();
+        __emscripten_proxy_dlsync_async(pthread_ptr, info.id);
+        PThread.outstandingPromises[pthread_ptr] = info;
+        promises.push(info.promise);
+      }
+    }
+
+#if PTHREADS_DEBUG
+    dbg('_emscripten_dlsync_threads_async: waiting on ' + promises.length + ' promises');
+#endif
+    // Once all promises are resolved then we know all threads are in sync and
+    // we can call the callback.
+    Promise.all(promises).then(() => {
+      PThread.outstandingPromises = {};
+#if PTHREADS_DEBUG
+      dbg('_emscripten_dlsync_threads_async done: calling callback');
+#endif
+      {{{ makeDynCall('vp', 'callback') }}}(ctx);
+    });
+  },
+
+  // Synchronous version dlsync_threads. This is only needed for the case then
+  // the main thread call dlopen and in that case we have not choice but to
+  // synchronously block the main thread until all other threads are in sync.
+  // When `dlopen` is called from a worker, the worker itself is blocked but
+  // the operation its waiting on (on the main thread) can be async.
+  _emscripten_dlsync_threads__deps: ['_emscripten_proxy_dlsync'],
+  _emscripten_dlsync_threads: function() {
+#if ASSERTIONS
+    assert(!ENVIRONMENT_IS_PTHREAD, 'Internal Error! _emscripten_dlsync_threads() can only ever be called from main thread');
+#endif
+    for (const ptr of Object.keys(PThread.pthreads)) {
+      const pthread_ptr = Number(ptr);
+      if (!PThread.finishedThreads.has(pthread_ptr)) {
+        __emscripten_proxy_dlsync(pthread_ptr);
+      }
+    }
+  },
+#endif // MAIN_MODULE
+
   $executeNotifiedProxyingQueue: function(queue) {
     // Set the notification state to processing.
     Atomics.store(HEAP32, queue >> 2, {{{ cDefine('NOTIFICATION_RECEIVED') }}});

system/include/emscripten/threading.h

@@ -275,9 +275,6 @@ int emscripten_pthread_attr_settransferredcanvases(pthread_attr_t *a, const char
 // blocking is not enabled, see ALLOW_BLOCKING_ON_MAIN_THREAD.
 void emscripten_check_blocking_allowed(void);
 
-// Experimental API for syncing loaded code between pthreads.
-void _emscripten_thread_sync_code();
-
 #ifdef __cplusplus
 }
 #endif