diff --git a/src/lib/libpthread.js b/src/lib/libpthread.js index df4687930deaa..3459aa3cedabe 100644 --- a/src/lib/libpthread.js +++ b/src/lib/libpthread.js @@ -116,6 +116,9 @@ var LibraryPThread = { // starting the next thread is faster. unusedWorkers: [], tlsInitFunctions: [], +#if MAIN_MODULE + tlsFreeFunctions: [], +#endif // Maps pthread_t pointers to the workers on which they are running. For // the reverse mapping, each worker has a `pthread_ptr` when its running a // pthread. @@ -699,6 +702,10 @@ var LibraryPThread = { // Register this function so that its gets called for each thread on // startup. PThread.tlsInitFunctions.push(tlsInitWrapper); + var tlsFreeFunc = moduleExports?.['_emscripten_tls_free']; + if (tlsFreeFunc) { + PThread.tlsFreeFunctions.push(tlsFreeFunc); + } // If the main thread is already running we also need to call this function // now. If the main thread is not yet running this will happen when it @@ -1223,6 +1230,8 @@ var LibraryPThread = { else postMessage({ cmd: {{{ CMD_MARK_AS_FINISHED }}}, thread }); }, + _emscripten_thread_free_tls: () => PThread.tlsFreeFunctions.forEach((f) => f()), + $markAsFinished: (pthread_ptr) => { #if PTHREADS_DEBUG dbg(`markAsFinished: ${ptrToString(pthread_ptr)}`); diff --git a/src/lib/libsigs.js b/src/lib/libsigs.js index f73ba4919dbd8..c769101a9cbeb 100644 --- a/src/lib/libsigs.js +++ b/src/lib/libsigs.js @@ -349,6 +349,7 @@ sigs = { _emscripten_system__sig: 'ip', _emscripten_thread_cleanup__sig: 'vp', _emscripten_thread_exit_joinable__sig: 'vp', + _emscripten_thread_free_tls__sig: 'v', _emscripten_thread_mailbox_await__sig: 'vp', _emscripten_thread_set_strongref__sig: 'vp', _emscripten_throw_longjmp__sig: 'v', diff --git a/system/lib/pthread/pthread_create.c b/system/lib/pthread/pthread_create.c index b4b0c7e1da8ff..d88b7b1efd941 100644 --- a/system/lib/pthread/pthread_create.c +++ b/system/lib/pthread/pthread_create.c @@ -366,6 +366,11 @@ void _emscripten_thread_exit(void* result) { __tl_unlock(); +#ifdef EMSCRIPTEN_DYNAMIC_LINKING + // Free the TLS data of all loaded side modules. + _emscripten_thread_free_tls(); +#endif + // Not hosting a pthread anymore in this worker set __pthread_self to NULL __set_thread_state(NULL, 0, 0, 1); diff --git a/system/lib/pthread/threading_internal.h b/system/lib/pthread/threading_internal.h index 5667129e6e48c..6a113ebfd3fa4 100644 --- a/system/lib/pthread/threading_internal.h +++ b/system/lib/pthread/threading_internal.h @@ -80,6 +80,7 @@ void _emscripten_thread_set_strongref(pthread_t thread); int _emscripten_thread_is_valid(pthread_t thread); void _emscripten_thread_exit_joinable(pthread_t thread); +void _emscripten_thread_free_tls(void); void _emscripten_thread_exit(void* result); void _emscripten_process_dlopen_queue(void); extern em_proxying_queue* _Atomic _dlopen_proxying_queue; diff --git a/test/core/pthread/test_pthread_dylink_tls_free.c b/test/core/pthread/test_pthread_dylink_tls_free.c new file mode 100644 index 0000000000000..ed2b07f2e6428 --- /dev/null +++ b/test/core/pthread/test_pthread_dylink_tls_free.c @@ -0,0 +1,47 @@ +#include +#include +#include +#include +#include + +#define NUM_THREADS 100 + +typedef int (*touch_side_tls_type)(int); + +// Kept static so it does not clash with the side module's exported +// touch_side_tls symbol under MAIN_MODULE. +static touch_side_tls_type touch_side_tls; + +void* thread_main(void* arg) { + assert(touch_side_tls(1) == 1); + return NULL; +} + +void run_threads(int count) { + for (int i = 0; i < count; i++) { + pthread_t thread; + pthread_create(&thread, NULL, thread_main, NULL); + pthread_join(thread, NULL); + } +} + +int main() { + void* handle = dlopen("libside.so", RTLD_NOW); + assert(handle); + touch_side_tls = (touch_side_tls_type)dlsym(handle, "touch_side_tls"); + assert(touch_side_tls); + + // Each thread allocates the side module's 4 KiB TLS block on first access and + // must free it when it exits. Without the fix the heap grows by one block + // per thread; with it, growth is bounded by the last thread's not-yet- + // reclaimed stack, far below NUM_THREADS blocks. NUM_THREADS is large so a + // real leak clearly exceeds that fixed slack. + run_threads(10); + size_t before = mallinfo().uordblks; + run_threads(NUM_THREADS); + size_t after = mallinfo().uordblks; + assert(after < before + NUM_THREADS * 4096 / 2); + + printf("done\n"); + return 0; +} diff --git a/test/core/pthread/test_pthread_dylink_tls_free_robust.c b/test/core/pthread/test_pthread_dylink_tls_free_robust.c new file mode 100644 index 0000000000000..6865e4d39f1f4 --- /dev/null +++ b/test/core/pthread/test_pthread_dylink_tls_free_robust.c @@ -0,0 +1,34 @@ +#include +#include +#include +#include +#include + +static void* (*leave_locked)(void); + +static void* thread_main(void* arg) { return leave_locked(); } + +int main() { + void* handle = dlopen("libside.so", RTLD_NOW); + assert(handle); + leave_locked = dlsym(handle, "leave_locked"); + assert(leave_locked); + + for (int i = 0; i < 10; ++i) { + pthread_t thread; + assert(pthread_create(&thread, NULL, thread_main, NULL) == 0); + void* guard; + assert(pthread_join(thread, &guard) == 0); + + // Robust mutex cleanup must finish before freeing the TLS containing the + // mutex. Otherwise it writes into the freed block and corrupts the heap. + volatile char* block = malloc(4096); + assert(block); + block[0] = 0x55; + free((void*)block); + free(guard); + } + + puts("done"); + return 0; +} diff --git a/test/core/pthread/test_pthread_dylink_tls_free_robust_side.c b/test/core/pthread/test_pthread_dylink_tls_free_robust_side.c new file mode 100644 index 0000000000000..b729ab662e5a6 --- /dev/null +++ b/test/core/pthread/test_pthread_dylink_tls_free_robust_side.c @@ -0,0 +1,22 @@ +#include +#include +#include + +static _Thread_local struct { + pthread_mutex_t mutex; + char padding[4096]; +} tls; + +void* leave_locked(void) { + pthread_mutexattr_t attr; + assert(pthread_mutexattr_init(&attr) == 0); + assert(pthread_mutexattr_setrobust(&attr, PTHREAD_MUTEX_ROBUST) == 0); + assert(pthread_mutex_init(&tls.mutex, &attr) == 0); + assert(pthread_mutex_lock(&tls.mutex) == 0); + + // Keep the following block allocated so that freeing TLS cannot merge it + // with the top of the heap. Leave the mutex locked for thread-exit cleanup. + void* guard = malloc(32); + assert(guard); + return guard; +} diff --git a/test/core/pthread/test_pthread_dylink_tls_free_side.c b/test/core/pthread/test_pthread_dylink_tls_free_side.c new file mode 100644 index 0000000000000..2450dd118c7fd --- /dev/null +++ b/test/core/pthread/test_pthread_dylink_tls_free_side.c @@ -0,0 +1,6 @@ +_Thread_local char side_tls[4096]; + +int touch_side_tls(int value) { + side_tls[0] = value; + return side_tls[0]; +} diff --git a/test/test_core.py b/test/test_core.py index 7d7785b77b47c..1a5c5caebb9ba 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -9441,6 +9441,28 @@ def test_pthread_dylink_tls(self): self.cflags += ['-Wno-experimental', '-pthread'] self.dylink_testf(test_file('core/pthread/test_pthread_dylink_tls.c')) + @needs_dylink + @requires_pthreads + def test_pthread_dylink_tls_free(self): + self.cflags += ['-Wno-experimental', '-pthread'] + self.build_dlfcn_lib(test_file('core/pthread/test_pthread_dylink_tls_free_side.c')) + + self.prep_dlfcn_main() + self.set_setting('EXIT_RUNTIME') + self.set_setting('PROXY_TO_PTHREAD') + self.do_runf('core/pthread/test_pthread_dylink_tls_free.c', 'done\n') + + @needs_dylink + @requires_pthreads + def test_pthread_dylink_tls_free_robust(self): + self.cflags += ['-Wno-experimental', '-pthread'] + self.build_dlfcn_lib(test_file('core/pthread/test_pthread_dylink_tls_free_robust_side.c')) + + self.prep_dlfcn_main() + self.set_setting('EXIT_RUNTIME') + self.set_setting('PROXY_TO_PTHREAD') + self.do_runf('core/pthread/test_pthread_dylink_tls_free_robust.c', 'done\n') + @needs_dylink @requires_pthreads def test_pthread_dylink_longjmp(self): diff --git a/tools/link.py b/tools/link.py index 062816b97464c..ee0b379eec986 100644 --- a/tools/link.py +++ b/tools/link.py @@ -1790,7 +1790,7 @@ def get_full_import_name(name): if settings.SIDE_MODULE: # For side modules, we ignore all REQUIRED_EXPORTS that might have been added above. # They all come from either libc or compiler-rt. The exception is __wasm_call_ctors - # and _emscripten_tls_init which are per-module exports. + # and _emscripten_tls_init/_emscripten_tls_free which are per-module exports. settings.REQUIRED_EXPORTS.clear() if not settings.STANDALONE_WASM: @@ -1798,6 +1798,8 @@ def get_full_import_name(name): settings.REQUIRED_EXPORTS.append('__wasm_call_ctors') if settings.PTHREADS: settings.REQUIRED_EXPORTS.append('_emscripten_tls_init') + if settings.SIDE_MODULE: + settings.REQUIRED_EXPORTS.append('_emscripten_tls_free') if settings.NODERAWFS: default_setting('NODE_HOST_ENV', 1)