diff --git a/src/library_pthread.js b/src/library_pthread.js index 54a46be18ee28..0ff53df4e3cf2 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -8,8 +8,7 @@ var LibraryPThread = { $PThread__postset: 'if (!ENVIRONMENT_IS_PTHREAD) PThread.initMainThreadBlock();', $PThread__deps: ['_emscripten_thread_init', 'emscripten_futex_wake', '$killThread', - '$cancelThread', '$cleanupThread', - '$freeThreadData', + '$cancelThread', 'exit', #if !MINIMAL_RUNTIME '$handleException', @@ -168,8 +167,6 @@ var LibraryPThread = { // dynamically grow. PThread.unusedWorkers.push(worker); PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(worker), 1); - // Not a running Worker anymore - freeThreadData(worker.pthread); // Detach the worker from the pthread object, and return it to the // worker pool as an unused worker. worker.pthread = undefined; @@ -255,8 +252,6 @@ var LibraryPThread = { _emscripten_main_thread_process_queued_calls(); } else if (cmd === 'spawnThread') { spawnThread(d); - } else if (cmd === 'cleanupThread') { - cleanupThread(d['thread']); } else if (cmd === 'killThread') { killThread(d['thread']); } else if (cmd === 'cancelThread') { @@ -275,11 +270,9 @@ var LibraryPThread = { err('Thread ' + d['threadId'] + ': ' + d['text']); } else if (cmd === 'alert') { alert('Thread ' + d['threadId'] + ': ' + d['text']); - } else if (cmd === 'detachedExit') { + } else if (cmd === 'threadDone') { #if ASSERTIONS assert(worker.pthread); - var detach_state = Atomics.load(HEAPU32, (worker.pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.detach_state }}}) >> 2); - assert(detach_state == {{{ cDefine('DT_EXITED') }}}); #endif PThread.returnWorkerToPool(worker); } else if (cmd === 'cancelDone') { @@ -311,7 +304,7 @@ var LibraryPThread = { worker.on('error', function(e) { worker.onerror(e); }); - worker.on('detachedExit', function() { + worker.on('threadDone', function() { // TODO: update the worker queue? // See: https://github.com/emscripten-core/emscripten/issues/9763 }); @@ -429,27 +422,6 @@ var LibraryPThread = { } }, - $freeThreadData__noleakcheck: true, - $freeThreadData: function(pthread) { -#if ASSERTIONS - assert(!ENVIRONMENT_IS_PTHREAD, 'Internal Error! freeThreadData() can only ever be called from main application thread!'); -#endif - if (!pthread) return; - if (pthread.threadInfoStruct) { -#if PTHREADS_PROFILING - var profilerBlock = {{{ makeGetValue('pthread.threadInfoStruct', C_STRUCTS.pthread.profilerBlock, 'i32') }}}; - {{{ makeSetValue('pthread.threadInfoStruct', C_STRUCTS.pthread.profilerBlock, 0, 'i32') }}}; - _free(profilerBlock); -#endif - _free(pthread.threadInfoStruct); - } - pthread.threadInfoStruct = 0; - if (pthread.allocatedOwnStack && pthread.stackBase) _free(pthread.stackBase); - pthread.stackBase = 0; - if (pthread.worker) pthread.worker.pthread = null; - }, - - $killThread__desp: ['$freeThreadData'], $killThread: function(pthread_ptr) { #if PTHREADS_DEBUG err('killThread 0x' + pthread_ptr.toString(16)); @@ -462,45 +434,12 @@ var LibraryPThread = { var pthread = PThread.pthreads[pthread_ptr]; delete PThread.pthreads[pthread_ptr]; pthread.worker.terminate(); - freeThreadData(pthread); // The worker was completely nuked (not just the pthread execution it was hosting), so remove it from running workers // but don't put it back to the pool. PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(pthread.worker), 1); // Not a running Worker anymore. pthread.worker.pthread = undefined; }, - __emscripten_thread_cleanup: function(thread) { - if (!ENVIRONMENT_IS_PTHREAD) cleanupThread(thread); - else postMessage({ 'cmd': 'cleanupThread', 'thread': thread }); - }, - - $cleanupThread: function(pthread_ptr) { -#if ASSERTIONS - assert(!ENVIRONMENT_IS_PTHREAD, 'Internal Error! cleanupThread() can only ever be called from main application thread!'); - assert(pthread_ptr, 'Internal Error! Null pthread_ptr in cleanupThread!'); -#endif - var pthread = PThread.pthreads[pthread_ptr]; - // If pthread has been removed from this map this also means that pthread_ptr points - // to already freed data. Such situation may occur in following circumstances: - // 1. Joining cancelled thread - in such situation it may happen that pthread data will - // already be removed by handling 'cancelDone' message. - // 2. Joining thread from non-main browser thread (this also includes thread running main() - // when compiled with `PROXY_TO_PTHREAD`) - in such situation it may happen that following - // code flow occur (MB - Main Browser Thread, S1, S2 - Worker Threads): - // S2: thread ends, 'exit' message is sent to MB - // S1: calls pthread_join(S2), this causes: - // a. S2 is marked as detached, - // b. 'cleanupThread' message is sent to MB. - // MB: handles 'exit' message, as thread is detached, so returnWorkerToPool() - // is called and all thread related structs are freed/released. - // MB: handles 'cleanupThread' message which calls this function. - if (pthread) { - {{{ makeSetValue('pthread_ptr', C_STRUCTS.pthread.self, 0, 'i32') }}}; - var worker = pthread.worker; - PThread.returnWorkerToPool(worker); - } - }, - $registerTlsInit: function(tlsInitFunc, moduleExports, metadata) { #if DYLINK_DEBUG err("registerTlsInit: " + tlsInitFunc); @@ -576,7 +515,6 @@ var LibraryPThread = { stackBase: threadParams.stackBase, stackSize: threadParams.stackSize, initialState: {{{ cDefine('DT_JOINABLE') }}}, - allocatedOwnStack: threadParams.allocatedOwnStack, // Info area for this thread in Emscripten HEAP (shared) threadInfoStruct: threadParams.pthread_ptr }; @@ -791,12 +729,13 @@ var LibraryPThread = { // stack size if not specified is 2 MB, so follow that convention. stackSize = {{{ DEFAULT_PTHREAD_STACK_SIZE }}}; } - // If allocatedOwnStack == true, then the pthread impl maintains the stack allocation. - var allocatedOwnStack = stackBase == 0; - if (allocatedOwnStack) { + // If stackBase is zero then we allocate a new stack region and mark it as + // owned. + if (stackBase == 0) { // Allocate a stack if the user doesn't want to place the stack in a // custom memory area. stackBase = _memalign({{{ STACK_ALIGN }}}, stackSize); + Atomics.store(HEAPU32, (pthread_ptr + {{{ C_STRUCTS.pthread.stack_owned }}}) >> 2, 1); } else { // Musl stores the stack base address assuming stack grows downwards, so // adjust it to Emscripten convention that the @@ -817,7 +756,6 @@ var LibraryPThread = { var threadParams = { stackBase: stackBase, stackSize: stackSize, - allocatedOwnStack: allocatedOwnStack, initialState: initialState, startRoutine: start_routine, pthread_ptr: pthread_ptr, @@ -910,10 +848,12 @@ var LibraryPThread = { return 0; }, - __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' }); + _emscripten_thread_done: function() { + // Marks the current thread as complete and allows the worker to be + // returned to the pool + // Called at the end of _emscripten_thread_exit which is called + // during pthread_exit or when a thread returns from its entry point. + postMessage({ 'cmd': 'threadDone' }); }, // Returns 0 on success, or one of the values -ETIMEDOUT, -EWOULDBLOCK or -EINVAL on error. diff --git a/src/struct_info_internal.json b/src/struct_info_internal.json index 939d8f70d2379..6e38ce2099b54 100644 --- a/src/struct_info_internal.json +++ b/src/struct_info_internal.json @@ -12,6 +12,7 @@ "detach_state", "stack", "stack_size", + "stack_owned", "result", "robust_list", "tid", diff --git a/system/lib/libc/musl/src/internal/pthread_impl.h b/system/lib/libc/musl/src/internal/pthread_impl.h index 3146faa3f91e1..a6e35cd402899 100644 --- a/system/lib/libc/musl/src/internal/pthread_impl.h +++ b/system/lib/libc/musl/src/internal/pthread_impl.h @@ -91,6 +91,7 @@ struct pthread { // If --threadprofiler is enabled, this pointer is allocated to contain // internal information about the thread state for profiling purposes. thread_profiler_block * _Atomic profilerBlock; + int stack_owned; #endif }; diff --git a/system/lib/pthread/pthread_create.c b/system/lib/pthread/pthread_create.c index 0038eeae9e34f..e5ea46e6faca5 100644 --- a/system/lib/pthread/pthread_create.c +++ b/system/lib/pthread/pthread_create.c @@ -22,7 +22,7 @@ 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_detached_exit(); +extern void _emscripten_thread_done(); extern void* _emscripten_tls_base(); extern int8_t __dso_handle; @@ -138,6 +138,19 @@ int __pthread_create(pthread_t* restrict res, return 0; } +/* + * Thread data is free'd in exactly two different places: + * 1. When a detached thread exits (_emscripten_thread_exit) + * 2. When joining a non-detached thread (pthread_join) + */ +void __pthread_free_thread_data(pthread_t t) { + if (t->profilerBlock) free(t->profilerBlock); + if (t->stack_owned) free(t->stack); + // To aid is debugging set all fields to zero + memset(t, 0, sizeof(*t)); + free(t); +} + static void free_tls_data() { void* tls_block = _emscripten_tls_base(); if (tls_block) { @@ -196,13 +209,18 @@ void _emscripten_thread_exit(void* result) { // When we publish this, the main thread is free to deallocate the thread // object and we are done. if (state == DT_DETACHED) { - self->detach_state = DT_EXITED; - __pthread_detached_exit(); + // If the thread was detached then we can free the thread data now, + // otherwise is stays around in another thread calls pthread_join. + __pthread_free_thread_data(self); } else { self->detach_state = DT_EXITING; // wake any threads that might be waiting for us to exit emscripten_futex_wake(&self->detach_state, INT_MAX); } + + // This will tell the main thread that we are done running on a thread + // on this worker and it can be returned to the worker pool. + _emscripten_thread_done(); } // Mark as `no_sanitize("address"` since emscripten_pthread_exit destroys diff --git a/system/lib/pthread/pthread_join.c b/system/lib/pthread/pthread_join.c index efa5990168439..950e97f607d61 100644 --- a/system/lib/pthread/pthread_join.c +++ b/system/lib/pthread/pthread_join.c @@ -10,7 +10,7 @@ #include extern int __pthread_join_js(pthread_t t, void **res, int tryjoin); -extern int __emscripten_thread_cleanup(pthread_t t); +extern void __pthread_free_thread_data(pthread_t t); static int __pthread_join_internal(pthread_t t, void **res) { if (t->self != t) { @@ -39,7 +39,7 @@ static int __pthread_join_internal(pthread_t t, void **res) { if (old_state == DT_EXITING) { // We successfully marked the tread as DT_EXITED if (res) *res = t->result; - __emscripten_thread_cleanup(t); + __pthread_free_thread_data(t); return 0; } assert(old_state == DT_JOINABLE); diff --git a/tests/reference_struct_info.json b/tests/reference_struct_info.json index 0786f571909fb..577ffed8e0ca0 100644 --- a/tests/reference_struct_info.json +++ b/tests/reference_struct_info.json @@ -1327,7 +1327,7 @@ "p_proto": 8 }, "pthread": { - "__size__": 108, + "__size__": 112, "cancel": 28, "cancelasync": 33, "canceldisable": 32, @@ -1338,6 +1338,7 @@ "robust_list": 68, "self": 0, "stack": 44, + "stack_owned": 108, "stack_size": 48, "tid": 16, "tsd": 64