Free a side module's file data once all threads have loaded it

ChangeLog.md

@@ -20,6 +20,10 @@ See docs/process.md for more on how version tagging works.
 
 6.0.14 (in development)
 -----------------------
+- In threaded builds, `dlopen` on the main thread no longer keeps a copy of the
+  side module's file in the heap after every thread has loaded it.  Threads
+  started later on existing workers now instantiate the already compiled
+  module instead of compiling it again.
 - When compiling and linking in a single command, auxiliary compiler output
   files (such as `.dwo` files from `-gsplit-dwarf` or `.json` files from
   `-ftime-trace`) are now named `<output>-<input>.<ext>`, matching the behavior

src/lib/libpthread.js

@@ -735,6 +735,12 @@ var LibraryPThread = {
         start_routine: threadParams.startRoutine,
         arg: threadParams.arg,
         pthread_ptr: threadParams.pthread_ptr,
+#if MAIN_MODULE
+        // The worker may have been created before some of these modules were
+        // loaded.  The thread loads them from here rather than from the file
+        // data in the dso, which isn't kept.
+        sharedModules,
+#endif
     };
 #if OFFSCREENCANVAS_SUPPORT
     // Note that we do not need to quote these names because they are only used
@@ -1291,14 +1297,18 @@ var LibraryPThread = {
   _emscripten_dlsync_threads: () => {
     const callingThread = PThread.currentProxiedOperationCallerThread;
     if (callingThread) {
-      return dlsyncThreadsAsync();
+      // Threads spawned while this runs may not see the new modules in
+      // `sharedModules`, so don't report that every thread is in sync.
+      return dlsyncThreadsAsync().then(() => 0);
     }
+    var synced = 1;
     for (const ptr of Object.keys(PThread.pthreads)) {
       const pthread_ptr = Number(ptr);
-      if (!PThread.finishedThreads.has(pthread_ptr)) {
-        __emscripten_proxy_dlsync(pthread_ptr);
+      if (!PThread.finishedThreads.has(pthread_ptr) && !__emscripten_proxy_dlsync(pthread_ptr)) {
+        synced = 0;
       }
     }
+    return synced;
   },
 #endif // MAIN_MODULE
 

src/lib/libsigs.js

@@ -330,7 +330,7 @@ sigs = {
   _emscripten_create_audio_worklet__sig: 'viipippp',
   _emscripten_create_wasm_worker__sig: 'iipip',
   _emscripten_dlopen_js__sig: 'vpppp',
-  _emscripten_dlsync_threads__sig: 'v',
+  _emscripten_dlsync_threads__sig: 'i',
   _emscripten_fetch_get_response_headers__sig: 'pipp',
   _emscripten_fetch_get_response_headers_length__sig: 'pi',
   _emscripten_fs_load_embedded_files__sig: 'vp',

src/runtime_pthread.js

@@ -16,7 +16,8 @@ var workerID = 0;
 
 #if MAIN_MODULE
 // Map of modules to be shared with new threads.  This gets populated by the
-// main thread and shared with all new workers via the initial `load` message.
+// main thread and sent to workers in the `load` message, and again in each
+// `run` message for modules loaded since.
 var sharedModules = {};
 #endif
 
@@ -134,6 +135,9 @@ if (ENVIRONMENT_IS_PTHREAD) {
 #if ASSERTIONS
         assert(msgData.pthread_ptr);
         assert(wasmMemory, "CMD_RUN received before CMD_LOAD");
+#endif
+#if MAIN_MODULE
+        sharedModules = msgData.sharedModules;
 #endif
         // Call inside JS module to set up the stack frame for this pthread in JS module scope.
         // This needs to be the first thing that we do, as we cannot call to any C/C++ functions

system/lib/libc/dynlink.c

@@ -156,12 +156,19 @@ static void load_library_done(struct dso* p) {
   new_dlevent(p, -1);
 #ifdef _REENTRANT
   // Block until all other threads have loaded this module.
-  _emscripten_dlsync_threads();
+  bool synced = _emscripten_dlsync_threads();
+  // When the main runtime thread loaded the module and every running thread
+  // has now loaded it too, nothing needs the file data any more: threads
+  // started from now on get the compiled module from `sharedModules` (see
+  // spawnThread).  Otherwise keep it: when another thread loaded the module
+  // the main thread catches up asynchronously, so a thread spawned meanwhile
+  // may still read it, and a thread that failed to load it may retry.
+  if (synced && emscripten_is_main_runtime_thread()) {
+    free(p->file_data);
+    p->file_data = NULL;
+    p->file_data_size = 0;
+  }
 #endif
-  // TODO: figure out some way to tell when its safe to free p->file_data.  Its
-  // not safe to do here because some threads could have been asleep then when
-  // the "dlsync" occurred and those threads will synchronize when they wake,
-  // which could be an arbitrarily long time in the future.
 }
 
 static struct dso* load_library_start(const char* name, int flags) {
@@ -177,8 +184,9 @@ static struct dso* load_library_start(const char* name, int flags) {
   strcpy(p->name, name);
 
   // If the file exists in the filesystem, load it here into linear memory which
-  // makes the data available to JS, and to other threads.  This data gets
-  // free'd later once all threads have loaded the DSO.
+  // makes the data available to JS, and to other threads.  load_library_done
+  // frees it once all threads have loaded the DSO, when it can tell that they
+  // have.
   struct stat statbuf;
   if (stat(name, &statbuf) == 0 && S_ISREG(statbuf.st_mode)) {
     int fd = open(name, O_RDONLY);
@@ -192,6 +200,7 @@ static struct dso* load_library_start(const char* name, int flags) {
             p->file_data_size = size;
           } else {
             free(p->file_data);
+            p->file_data = NULL;
           }
         }
       }
@@ -407,6 +416,8 @@ int _emscripten_proxy_dlsync_async(pthread_t target_thread, em_promise_t promise
   return rtn;
 }
 
+// Returns false if the target thread failed to load the modules.  A thread
+// that has exited doesn't need them, so that counts as success.
 int _emscripten_proxy_dlsync(pthread_t target_thread) {
   assert(emscripten_is_main_runtime_thread());
   if (!_dlopen_proxying_queue) {
@@ -415,7 +426,7 @@ int _emscripten_proxy_dlsync(pthread_t target_thread) {
   int result;
   if (!emscripten_proxy_sync(
         _dlopen_proxying_queue, target_thread, do_thread_sync_out, &result)) {
-    return 0;
+    return 1;
   }
   return result;
 }
@@ -438,6 +449,7 @@ static void dlopen_onerror(struct dso* dso, void* user_data) {
   dbg("dlopen_js_onerror: dso=%p", dso);
   do_write_unlock();
   data->onerror(data->user_data);
+  free(dso->file_data);
   free(dso);
   free(data);
 }
@@ -543,6 +555,7 @@ static struct dso* _dlopen(const char* file, int flags) {
   void* success = _dlopen_js(p);
   if (!success) {
     dbg("dlopen_js: failed: %p", p);
+    free(p->file_data);
     free(p);
     p = NULL;
     goto end;

system/lib/libc/emscripten_internal.h

@@ -100,7 +100,10 @@ int _setitimer_js(int which, double timeout);
 // Synchronize loaded modules across threads.
 // Runs _emscripten_dlsync_self on each of the threads that are running at
 // the time of the call.
-void _emscripten_dlsync_threads();
+// Returns true only when called on the main runtime thread outside of a
+// proxied call, and every running thread loaded the modules: threads spawned
+// from then on get them from `sharedModules`.
+bool _emscripten_dlsync_threads();
 
 #ifdef _GNU_SOURCE
 void __call_sighandler(sighandler_t handler, int sig);

test/codesize/test_codesize_hello_dylink_all.json

@@ -1,7 +1,7 @@
 {
   "a.out.js": 272704,
-  "a.out.nodebug.wasm": 589443,
-  "total": 862147,
+  "a.out.nodebug.wasm": 589466,
+  "total": 862170,
   "sent": [
     "IMG_Init",
     "IMG_Load",

test/core/pthread/test_pthread_dlopen_file_data.c

@@ -0,0 +1,70 @@
+// In threaded builds dlopen copies the side module's file into linear memory
+// so that other threads can load it.  Check that the copy is freed once every
+// running thread has loaded the module, and that threads started afterwards can
+// still load it, both from the main thread and from another thread, on workers
+// that were created before the dlopen.
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <malloc.h>
+#include <pthread.h>
+#include <stdatomic.h>
+#include <stdio.h>
+#include <unistd.h>
+
+typedef int (*side_fn_t)(int);
+static side_fn_t side_fn;
+static _Atomic int go;
+
+static void* nested_thread(void* arg) {
+  return (void*)(long)side_fn(3);
+}
+
+// Running while the main thread calls dlopen.  Once released, it calls into the
+// side module and starts another thread that does too.
+static void* running_thread(void* arg) {
+  while (!go) {
+    usleep(1000);
+  }
+  pthread_t nested;
+  void* result;
+  pthread_create(&nested, NULL, nested_thread, NULL);
+  pthread_join(nested, &result);
+  assert((long)result == 4);
+  return (void*)(long)side_fn(1);
+}
+
+// Started by the main thread after the dlopen.
+static void* later_thread(void* arg) {
+  return (void*)(long)side_fn(2);
+}
+
+int main() {
+  pthread_t running, later;
+  void* result;
+  pthread_create(&running, NULL, running_thread, NULL);
+
+  size_t used = mallinfo().uordblks;
+  void* h = dlopen("/lib/libside.so", RTLD_NOW);
+  assert(h);
+  size_t growth = mallinfo().uordblks - used;
+  printf("heap growth: %zuKB\n", growth >> 10);
+  assert(growth < (1 << 20));
+  // Threads that load the module from now on can't fall back to the file.
+  unlink("/lib/libside.so");
+
+  side_fn = (side_fn_t)dlsym(h, "side_fn");
+  assert(side_fn && side_fn(0) == 1);
+
+  // Start this while the first thread is still running, so that it runs on a
+  // pool worker that hasn't loaded the module.
+  pthread_create(&later, NULL, later_thread, NULL);
+  pthread_join(later, &result);
+  assert((long)result == 3);
+
+  go = 1;
+  pthread_join(running, &result);
+  assert((long)result == 2);
+  printf("done\n");
+  return 0;
+}

test/test_core.py

@@ -9394,6 +9394,29 @@ def test_pthread_dlopen(self, args):
                  ['side module ctor', 'done\n', 'side module atexit'],
                  assert_all=True, cflags=args)
 
+  @needs_dylink
+  @requires_pthreads
+  def test_pthread_dlopen_file_data(self):
+    self.cflags += ['-Wno-experimental', '-pthread']
+    create_file('side.c', 'int side_fn(int x) { return x + 1; }')
+    self.build_dlfcn_lib('side.c')
+    # Pad the side module with an 8MB custom section so that the copy dlopen
+    # makes of it is easy to see in the heap.
+    name = b'padding'
+    payload = webassembly.to_leb(len(name)) + name + b'\0' * (8 * 1024 * 1024)
+    with open('libside.so', 'ab') as f:
+      f.write(b'\0' + webassembly.to_leb(len(payload)) + payload)
+    # Embed it under a path that doesn't exist on the host, so that threads
+    # can't read it from there either under node.
+    self.cflags += ['--embed-file', 'libside.so@/lib/libside.so']
+    self.prep_dlfcn_main()
+    self.set_setting('EXIT_RUNTIME')
+    # One worker each for the running, later and nested threads.
+    self.set_setting('PTHREAD_POOL_SIZE', 3)
+    if not self.get_setting('INITIAL_MEMORY'):
+      self.set_setting('INITIAL_MEMORY', '64mb')
+    self.do_runf('core/pthread/test_pthread_dlopen_file_data.c', 'done\n')
+
   @needs_dylink
   @requires_pthreads
   @flaky('https://github.com/emscripten-core/emscripten/issues/18887')