From 5662a6c92cbee46559a626c95e470e5e136c6828 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Thu, 16 Sep 2021 17:39:05 -0700 Subject: [PATCH] Enable file locking in musl stdio This is second attempt at landing a version of https://github.com/emscripten-core/emscripten/commit/d5d5f69ad066aee2e22a601921411b94206d035d The first time we tried it #13837 we ran into issues with test_pthread_exit_process deadlocking which I tracked down to an issue with `system/lib/libc/musl/src/thread/__wait.c` where it was blocking the main thread forever rather then looping and calling `emscripten_main_thread_process_queued_calls`. Includes a version of #14481 so that should land before this does. Fixes: #13194 --- system/lib/libc/musl/src/thread/__wait.c | 5 +- system/lib/pthread/pthread_create.c | 21 +++++++ tests/core/test_stdio_locking.c | 55 +++++++++++++++++++ tests/core/test_stdio_locking.out | 22 ++++++++ ...n_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports | 2 +- ...ain_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.funcs | 5 ++ ...n_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.imports | 1 + ...main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent | 1 + ...main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size | 2 +- tests/test_core.py | 6 ++ tools/system_libs.py | 2 +- 11 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 tests/core/test_stdio_locking.c create mode 100644 tests/core/test_stdio_locking.out diff --git a/system/lib/libc/musl/src/thread/__wait.c b/system/lib/libc/musl/src/thread/__wait.c index 265451dadb069..816f4c4a29a80 100644 --- a/system/lib/libc/musl/src/thread/__wait.c +++ b/system/lib/libc/musl/src/thread/__wait.c @@ -1,10 +1,13 @@ #ifdef __EMSCRIPTEN__ #include +#include #endif #include "pthread_impl.h" +#ifdef __EMSCRIPTEN__ int _pthread_isduecanceled(struct pthread *pthread_ptr); +#endif void __wait(volatile int *addr, volatile int *waiters, int val, int priv) { @@ -18,7 +21,7 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv) #ifdef __EMSCRIPTEN__ int is_main_thread = emscripten_is_main_runtime_thread(); while (*addr==val) { - if (pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) { + if (is_main_thread || pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) { // Must wait in slices in case this thread is cancelled in between. int e; do { diff --git a/system/lib/pthread/pthread_create.c b/system/lib/pthread/pthread_create.c index badbf3f0385ff..e277d372970e0 100644 --- a/system/lib/pthread/pthread_create.c +++ b/system/lib/pthread/pthread_create.c @@ -7,6 +7,7 @@ #define _GNU_SOURCE #include "pthread_impl.h" +#include "stdio_impl.h" #include "assert.h" #include #include @@ -53,6 +54,15 @@ void __do_cleanup_pop(struct __ptcb *cb) { __pthread_self()->cancelbuf = cb->__next; } +static FILE *volatile dummy_file = 0; +weak_alias(dummy_file, __stdin_used); +weak_alias(dummy_file, __stdout_used); +weak_alias(dummy_file, __stderr_used); + +static void init_file_lock(FILE *f) { + if (f && f->lock<0) f->lock = 0; +} + int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg) { // Note on LSAN: lsan intercepts/wraps calls to pthread_create so any // allocation we we do here should be considered leaks. @@ -61,6 +71,17 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att return EINVAL; } + pthread_t self = __pthread_self(); + if (!libc.threaded) { + for (FILE *f=*__ofl_lock(); f; f=f->next) + init_file_lock(f); + __ofl_unlock(); + init_file_lock(__stdin_used); + init_file_lock(__stdout_used); + init_file_lock(__stderr_used); + libc.threaded = 1; + } + // Allocate thread block (pthread_t structure). struct pthread *new = malloc(sizeof(struct pthread)); // zero-initialize thread structure. diff --git a/tests/core/test_stdio_locking.c b/tests/core/test_stdio_locking.c new file mode 100644 index 0000000000000..d8ddc512eb3ff --- /dev/null +++ b/tests/core/test_stdio_locking.c @@ -0,0 +1,55 @@ +/* + * Regression test for stdio locking. If file locking is not enabled the + * threads will race to write the file output buffer and we will see lines + * that are longer or shorter then 100 characters. When locking is + * working/enabled each 100 charactor line will be printed seperately. + * + * See: + * musl/src/stdio/__lockfile.c + * musl/src/stdio/fwrite.c + */ +#include +#include +#include +#include +#include + +pthread_t thread[2]; + +char *char_repeat(int n, char c) { + char *dest = malloc(n + 1); + memset(dest, c, n); + dest[n] = '\0'; + return dest; +} + +void *thread_main(void *arg) { + char *msg = char_repeat(100, 'a'); + for (int i = 0; i < 10; ++i) + printf("%s\n", msg); + free(msg); + return 0; +} + +int main() { + printf("in main\n"); + void *thread_rtn; + int rc; + + rc = pthread_create(&thread[0], NULL, thread_main, NULL); + assert(rc == 0); + + rc = pthread_create(&thread[1], NULL, thread_main, NULL); + assert(rc == 0); + + rc = pthread_join(thread[0], &thread_rtn); + assert(rc == 0); + assert(thread_rtn == 0); + + rc = pthread_join(thread[1], &thread_rtn); + assert(rc == 0); + assert(thread_rtn == 0); + + printf("main done\n"); + return 0; +} diff --git a/tests/core/test_stdio_locking.out b/tests/core/test_stdio_locking.out new file mode 100644 index 0000000000000..48d52e8763372 --- /dev/null +++ b/tests/core/test_stdio_locking.out @@ -0,0 +1,22 @@ +in main +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +main done diff --git a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports index 8bbfd29999464..c52195598c0ac 100644 --- a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports +++ b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.exports @@ -14,7 +14,7 @@ M N O P -v +Q w x y diff --git a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.funcs b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.funcs index 14bfd302ef5f3..d498639a6ab0b 100644 --- a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.funcs +++ b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.funcs @@ -1,11 +1,14 @@ $GetQueue $__emscripten_init_main_thread +$__emscripten_stdout_seek $__errno_location $__pthread_mutex_lock $__pthread_mutex_trylock $__pthread_mutex_unlock $__pthread_self_internal $__pthread_setcancelstate +$__stdio_write +$__wasi_syscall_ret $__wasm_call_ctors $__wasm_init_memory $_do_call @@ -34,9 +37,11 @@ $emscripten_sync_run_in_main_thread $emscripten_sync_run_in_main_thread $emscripten_tls_init $free_tls +$init_file_lock $init_mparams $main $memset +$pthread_mutexattr_destroy $sbrk $stackAlloc $stackRestore diff --git a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.imports b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.imports index 014f2e2290126..54aa456c94818 100644 --- a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.imports +++ b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.imports @@ -19,3 +19,4 @@ a.r a.s a.t a.u +a.v diff --git a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent index ad11ff431eda4..2e51e319a55ac 100644 --- a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent +++ b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.sent @@ -19,3 +19,4 @@ r s t u +v diff --git a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size index 2e4a9389fbafb..82161e9ab10c6 100644 --- a/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size +++ b/tests/other/metadce/minimal_main_Oz_USE_PTHREADS_PROXY_TO_PTHREAD.size @@ -1 +1 @@ -16368 +16977 diff --git a/tests/test_core.py b/tests/test_core.py index a3c7c5993a8ee..1f40ed8b97356 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -8406,6 +8406,12 @@ def test_emscripten_futexes(self): self.set_setting('USE_PTHREADS') self.do_run_in_out_file_test('core/pthread/emscripten_futexes.c') + @node_pthreads + def test_stdio_locking(self): + self.set_setting('PTHREAD_POOL_SIZE', '2') + self.set_setting('EXIT_RUNTIME') + self.do_run_in_out_file_test('core', 'test_stdio_locking.c') + @needs_dylink @node_pthreads def test_pthread_dylink_basics(self): diff --git a/tools/system_libs.py b/tools/system_libs.py index 018ab186acdd0..60199959ddcf5 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -1299,7 +1299,7 @@ class CompilerRTLibrary(Library): force_object_files = True -class libc_rt_wasm(OptimizedAggressivelyForSizeLibrary, AsanInstrumentedLibrary, CompilerRTLibrary, MuslInternalLibrary): +class libc_rt_wasm(OptimizedAggressivelyForSizeLibrary, AsanInstrumentedLibrary, CompilerRTLibrary, MuslInternalLibrary, MTLibrary): name = 'libc_rt_wasm' def get_files(self):