diff --git a/src/library_pthread.js b/src/library_pthread.js index 21f303a5bcdac..daa5db658f5c9 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -285,11 +285,13 @@ var LibraryPThread = { err('Thread ' + d['threadId'] + ': ' + d['text']); } else if (cmd === 'alert') { alert('Thread ' + d['threadId'] + ': ' + d['text']); - } else if (cmd === 'exit') { - var detached = worker.pthread && Atomics.load(HEAPU32, (worker.pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.detached }}}) >> 2); - if (detached) { - PThread.returnWorkerToPool(worker); - } + } else if (cmd === 'detachedExit') { +#if ASSERTIONS + assert(worker.pthread); + var detached = Atomics.load(HEAPU32, (worker.pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.detached }}}) >> 2); + assert(detached); +#endif + PThread.returnWorkerToPool(worker); } else if (cmd === 'exitProcess') { // A pthread has requested to exit the whole application process (runtime). #if ASSERTIONS @@ -331,7 +333,7 @@ var LibraryPThread = { worker.on('error', function(e) { worker.onerror(e); }); - worker.on('exit', function() { + worker.on('detachedExit', function() { // TODO: update the worker queue? // See: https://github.com/emscripten-core/emscripten/issues/9763 }); @@ -901,29 +903,14 @@ var LibraryPThread = { return 0; }, - __pthread_detach_js__sig: 'vi', - __pthread_detach_js: function(thread) { - if (!thread) { - err('pthread_detach attempted on a null thread pointer!'); - return {{{ cDefine('ESRCH') }}}; - } - var self = {{{ makeGetValue('thread', C_STRUCTS.pthread.self, 'i32') }}}; - if (self !== thread) { - err('pthread_detach attempted on thread ' + thread + ', which does not point to a valid thread, or does not exist anymore!'); - return {{{ cDefine('ESRCH') }}}; - } - // Follow musl convention: detached:0 means not detached, 1 means the thread - // was created as detached, and 2 means that the thread was detached via - // pthread_detach. - var wasDetached = Atomics.compareExchange(HEAPU32, (thread + {{{ C_STRUCTS.pthread.detached }}} ) >> 2, 0, 2); - - return wasDetached ? {{{ cDefine('EINVAL') }}} : 0; - }, - __pthread_exit_run_handlers__deps: ['exit'], __pthread_exit_run_handlers: function(status) { // Called from pthread_exit, either when called explicitly called // by programmer, or implicitly when leaving the thread main function. + // + // Note: in theory we would like to return any offscreen canvases back to + // the main thread, but if we ever fetched a rendering context for them that + // would not be valid, so we don't try. #if PTHREADS_DEBUG var tb = _pthread_self(); @@ -936,13 +923,10 @@ var LibraryPThread = { } }, - __pthread_exit_done: function() { - // Called at the end of pthread_exit, either when called explicitly called - // by programmer, or implicitly when leaving the thread main function. - // - // Note: in theory we would like to return any offscreen canvases back to the main thread, - // but if we ever fetched a rendering context for them that would not be valid, so we don't try. - postMessage({ 'cmd': 'exit' }); + __pthread_detached_exit: function() { + // Called at the end of pthread_exit (which occurs also when leaving the + // thread main function) if an only if the thread is in a detached state. + postMessage({ 'cmd': 'detachedExit' }); }, __cxa_thread_atexit__sig: 'vii', diff --git a/system/lib/libc/musl/src/thread/pthread_detach.c b/system/lib/libc/musl/src/thread/pthread_detach.c index ed77f74d520bf..263799504f00b 100644 --- a/system/lib/libc/musl/src/thread/pthread_detach.c +++ b/system/lib/libc/musl/src/thread/pthread_detach.c @@ -5,6 +5,14 @@ int __pthread_join(pthread_t, void **); static int __pthread_detach(pthread_t t) { + // XXX EMSCRIPTEN: Add check for invalid (already joined) thread. Again + // for the benefit of the conformance tests. + if (t->self != t) + return ESRCH; + // XXX EMSCRIPTEN: Even though the man page says this is undefined behaviour + // we have several tests in the posixtest suite that depend on this. + if (t->detached) + return EINVAL; /* Cannot detach a thread that's already exiting */ if (a_swap(t->exitlock, 1)) return __pthread_join(t, 0); @@ -15,3 +23,5 @@ static int __pthread_detach(pthread_t t) weak_alias(__pthread_detach, pthread_detach); weak_alias(__pthread_detach, thrd_detach); +// XXX EMSCRIPTEN: add extra alias for asan. +weak_alias(__pthread_detach, emscripten_builtin_pthread_detach); diff --git a/system/lib/pthread/pthread_create.c b/system/lib/pthread/pthread_create.c index 56038552af43d..badbf3f0385ff 100644 --- a/system/lib/pthread/pthread_create.c +++ b/system/lib/pthread/pthread_create.c @@ -20,7 +20,7 @@ extern int __cxa_thread_atexit(void (*)(void *), void *, void *); extern int __pthread_create_js(struct pthread *thread, const pthread_attr_t *attr, void *(*start_routine) (void *), void *arg); extern void _emscripten_thread_init(int, int, int); extern void __pthread_exit_run_handlers(); -extern void __pthread_exit_done(); +extern void __pthread_detached_exit(); extern int8_t __dso_handle; static void dummy_0() @@ -97,6 +97,8 @@ void _emscripten_thread_exit(void* result) { // Call into the musl function that runs destructors of all thread-specific data. __pthread_tsd_run_dtors(); + __lock(self->exitlock); + if (self == emscripten_main_browser_thread_id()) { // FIXME(sbc): When pthread_exit causes the entire application to exit // we should be returning zero (according to the man page for pthread_exit). @@ -112,13 +114,19 @@ void _emscripten_thread_exit(void* result) { // Not hosting a pthread anymore in this worker set __pthread_self to NULL _emscripten_thread_init(0, 0, 0); - // Mark the thread as no longer running. + // Cache deteched state since once we set threadStatus to 1, the `self` struct + // could be freed and reused. + int detatched = self->detached; + + // Mark the thread as no longer running so it can be joined. // Once we publish this, any threads that are waiting to join with us can - // proceed and this worker can be recycled and used to another thread. + // proceed and this worker can be recycled and used on another thread. self->threadStatus = 1; emscripten_futex_wake(&self->threadStatus, INT_MAX); // wake all threads - __pthread_exit_done(); + if (detatched) { + __pthread_detached_exit(); + } } // Mark as `no_sanitize("address"` since emscripten_pthread_exit destroys diff --git a/system/lib/pthread/pthread_detach.c b/system/lib/pthread/pthread_detach.c deleted file mode 100644 index 80584f527440b..0000000000000 --- a/system/lib/pthread/pthread_detach.c +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Copyright 2021 The Emscripten Authors. All rights reserved. - * Emscripten is available under two separate licenses, the MIT license and the - * University of Illinois/NCSA Open Source License. Both these licenses can be - * found in the LICENSE file. - */ - -#include "pthread_impl.h" -#include - -extern int __pthread_detach_js(pthread_t t); - -int __pthread_detach(pthread_t t) { - return __pthread_detach_js(t); -} - -weak_alias(__pthread_detach, emscripten_builtin_pthread_detach); -weak_alias(__pthread_detach, pthread_detach); -weak_alias(__pthread_detach, thrd_detach); diff --git a/tools/system_libs.py b/tools/system_libs.py index 1e63f0d47bbb9..5c7c2bac59aad 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -748,7 +748,7 @@ def get_files(self): 'lock_ptc.c', # 'pthread_setattr_default_np.c', # TODO: These could be moved away from JS in the upcoming musl upgrade. - 'pthread_cancel.c', 'pthread_detach.c', + 'pthread_cancel.c', 'pthread_join.c', 'pthread_testcancel.c', ] libc_files += files_in_path( @@ -756,7 +756,6 @@ def get_files(self): filenames=[ 'library_pthread.c', 'pthread_create.c', - 'pthread_detach.c', 'pthread_join.c', 'pthread_testcancel.c', 'emscripten_proxy_main.c',