diff --git a/ChangeLog.md b/ChangeLog.md index c2e9b8ac929fb..5511a768af1f9 100644 --- a/ChangeLog.md +++ b/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 `-.`, matching the behavior diff --git a/src/lib/libpthread.js b/src/lib/libpthread.js index df4687930deaa..2fcdd0e604673 100644 --- a/src/lib/libpthread.js +++ b/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 diff --git a/src/lib/libsigs.js b/src/lib/libsigs.js index f73ba4919dbd8..65598cbdfebe5 100644 --- a/src/lib/libsigs.js +++ b/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', diff --git a/src/runtime_pthread.js b/src/runtime_pthread.js index 27996fc16499e..5161a26cd651c 100644 --- a/src/runtime_pthread.js +++ b/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 diff --git a/system/lib/libc/dynlink.c b/system/lib/libc/dynlink.c index e2cf70df3e185..01aeaa44959a6 100644 --- a/system/lib/libc/dynlink.c +++ b/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; diff --git a/system/lib/libc/emscripten_internal.h b/system/lib/libc/emscripten_internal.h index 2f408d6c621fe..5515e152c8687 100644 --- a/system/lib/libc/emscripten_internal.h +++ b/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); diff --git a/test/codesize/test_codesize_hello_dylink_all.json b/test/codesize/test_codesize_hello_dylink_all.json index 4d4e9e1e31f10..9e00c1a4044d6 100644 --- a/test/codesize/test_codesize_hello_dylink_all.json +++ b/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", diff --git a/test/core/pthread/test_pthread_dlopen_file_data.c b/test/core/pthread/test_pthread_dlopen_file_data.c new file mode 100644 index 0000000000000..07d4f4cb77317 --- /dev/null +++ b/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 +#include +#include +#include +#include +#include +#include + +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; +} diff --git a/test/test_core.py b/test/test_core.py index 7d7785b77b47c..3d718a3f3e23d 100644 --- a/test/test_core.py +++ b/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')