diff --git a/src/library_pthread.js b/src/library_pthread.js index 81470bc17d294..e49372a4cd998 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -173,6 +173,30 @@ var LibraryPThread = { if (ENVIRONMENT_IS_PTHREAD && _pthread_self()) ___pthread_tsd_run_dtors(); }, + runExitHandlersAndDeinitThread: function(tb, exitCode) { +#if PTHREADS_PROFILING + var profilerBlock = Atomics.load(HEAPU32, (tb + {{{ C_STRUCTS.pthread.profilerBlock }}} ) >> 2); + Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.profilerBlock }}} ) >> 2, 0); + _free(profilerBlock); +#endif + + // Disable all cancellation so that executing the cleanup handlers won't trigger another JS + // canceled exception to be thrown. + Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.canceldisable }}} ) >> 2, 1/*PTHREAD_CANCEL_DISABLE*/); + Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.cancelasync }}} ) >> 2, 0/*PTHREAD_CANCEL_DEFERRED*/); + PThread.runExitHandlers(); + + Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.threadExitCode }}} ) >> 2, exitCode); + // When we publish this, the main thread is free to deallocate the thread object and we are done. + // Therefore set _pthread_self = 0; above to 'release' the object in this worker thread. + Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.threadStatus }}} ) >> 2, 1); // Mark the thread as no longer running. + + _emscripten_futex_wake(tb + {{{ C_STRUCTS.pthread.threadStatus }}}, {{{ cDefine('INT_MAX') }}}); // wake all threads + + // Not hosting a pthread anymore in this worker, reset the info structures to null. + __emscripten_thread_init(0, 0, 0); // Unregister the thread block also inside the asm.js scope. + }, + // Called when we are performing a pthread_exit(), either explicitly called // by programmer, or implicitly when leaving the thread main function. threadExit: function(exitCode) { @@ -181,24 +205,8 @@ var LibraryPThread = { #if ASSERTIONS err('Pthread 0x' + tb.toString(16) + ' exited.'); #endif -#if PTHREADS_PROFILING - var profilerBlock = Atomics.load(HEAPU32, (tb + {{{ C_STRUCTS.pthread.profilerBlock }}} ) >> 2); - Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.profilerBlock }}} ) >> 2, 0); - _free(profilerBlock); -#endif - Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.threadExitCode }}} ) >> 2, exitCode); - // When we publish this, the main thread is free to deallocate the thread object and we are done. - // Therefore set _pthread_self = 0; above to 'release' the object in this worker thread. - Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.threadStatus }}} ) >> 2, 1); - - // Disable all cancellation so that executing the cleanup handlers won't trigger another JS - // canceled exception to be thrown. - Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.canceldisable }}} ) >> 2, 1/*PTHREAD_CANCEL_DISABLE*/); - Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.cancelasync }}} ) >> 2, 0/*PTHREAD_CANCEL_DEFERRED*/); - PThread.runExitHandlers(); - - _emscripten_futex_wake(tb + {{{ C_STRUCTS.pthread.threadStatus }}}, {{{ cDefine('INT_MAX') }}}); - __emscripten_thread_init(0, 0, 0); // Unregister the thread block also inside the asm.js scope. + PThread.runExitHandlersAndDeinitThread(tb, exitCode); + if (ENVIRONMENT_IS_PTHREAD) { // 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. @@ -208,13 +216,7 @@ var LibraryPThread = { }, threadCancel: function() { - PThread.runExitHandlers(); - var tb = _pthread_self(); - Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.threadExitCode }}} ) >> 2, -1/*PTHREAD_CANCELED*/); - Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.threadStatus }}} ) >> 2, 1); // Mark the thread as no longer running. - _emscripten_futex_wake(tb + {{{ C_STRUCTS.pthread.threadStatus }}}, {{{ cDefine('INT_MAX') }}}); // wake all threads - // Not hosting a pthread anymore in this worker, reset the info structures to null. - __emscripten_thread_init(0, 0, 0); // Unregister the thread block also inside the asm.js scope. + PThread.runExitHandlersAndDeinitThread(_pthread_self(), -1/*PTHREAD_CANCELED*/); postMessage({ 'cmd': 'cancelDone' }); }, @@ -494,9 +496,23 @@ var LibraryPThread = { $cleanupThread: function(pthread_ptr) { if (ENVIRONMENT_IS_PTHREAD) throw 'Internal Error! cleanupThread() can only ever be called from main application thread!'; if (!pthread_ptr) throw 'Internal Error! Null pthread_ptr in cleanupThread!'; - {{{ makeSetValue('pthread_ptr', C_STRUCTS.pthread.self, 0, 'i32') }}}; 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); } diff --git a/system/lib/libc/musl/src/thread/__timedwait.c b/system/lib/libc/musl/src/thread/__timedwait.c index 71abe8e775b65..433f1d00c6300 100644 --- a/system/lib/libc/musl/src/thread/__timedwait.c +++ b/system/lib/libc/musl/src/thread/__timedwait.c @@ -42,7 +42,12 @@ int __timedwait_cp(volatile int *addr, int val, #ifdef __EMSCRIPTEN__ double msecsToSleep = top ? (top->tv_sec * 1000 + top->tv_nsec / 1000000.0) : INFINITY; int is_main_thread = emscripten_is_main_browser_thread(); - if (is_main_thread || pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) { + // cp suffix in the function name means "cancellation point", so this wait can be cancelled + // by the users unless current threads cancelability is set to PTHREAD_CANCEL_DISABLE + // which may be either done by the user of __timedwait() function. + if (is_main_thread || + pthread_self()->canceldisable != PTHREAD_CANCEL_DISABLE || + pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) { double sleepUntilTime = emscripten_get_now() + msecsToSleep; do { if (_pthread_isduecanceled(pthread_self())) { diff --git a/system/lib/libc/musl/src/thread/pthread_barrier_wait.c b/system/lib/libc/musl/src/thread/pthread_barrier_wait.c index 46b856619829a..8e212f0090803 100644 --- a/system/lib/libc/musl/src/thread/pthread_barrier_wait.c +++ b/system/lib/libc/musl/src/thread/pthread_barrier_wait.c @@ -87,14 +87,29 @@ int pthread_barrier_wait(pthread_barrier_t *b) while (spins-- && !inst->finished) a_spin(); a_inc(&inst->finished); - while (inst->finished == 1) { #ifdef __EMSCRIPTEN__ - emscripten_futex_wait(&inst->finished, 1, INFINITY); + int is_main_thread = emscripten_is_main_browser_thread(); + while (inst->finished == 1) { + if (is_main_thread) { + int e; + do { + // Main thread waits in _very_ small slices so that it stays responsive to assist proxied + // pthread calls. + e = emscripten_futex_wait(&inst->finished, 1, 1); + // Assist other threads by executing proxied operations that are effectively singlethreaded. + emscripten_main_thread_process_queued_calls(); + } while(e == -ETIMEDOUT); + } else { + // Can wait in one go. + emscripten_futex_wait(&inst->finished, 1, INFINITY); + } + } #else + while (inst->finished == 1) { __syscall(SYS_futex,&inst->finished,FUTEX_WAIT|128,1,0) != -ENOSYS || __syscall(SYS_futex,&inst->finished,FUTEX_WAIT,1,0); -#endif } +#endif return PTHREAD_BARRIER_SERIAL_THREAD; } diff --git a/tests/pthread/test_pthread_cancel_cond_wait.cpp b/tests/pthread/test_pthread_cancel_cond_wait.cpp new file mode 100644 index 0000000000000..a53ffa9459d45 --- /dev/null +++ b/tests/pthread/test_pthread_cancel_cond_wait.cpp @@ -0,0 +1,87 @@ +// 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 +#include +#include +#include +#include +#include +#include +#include +#include + +pthread_barrier_t barrier; +pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; +pthread_cond_t condvar = PTHREAD_COND_INITIALIZER; + +int th_cancelled = 0; + +volatile int res = 43; + +static void cleanup_handler(void *arg) { + emscripten_log(EM_LOG_CONSOLE, "Called clean-up handler with arg %p", arg); + int a = reinterpret_cast(arg); + res -= a; + + pthread_mutex_unlock(&mutex); + pthread_barrier_wait(&barrier); +} + +static void *thread_start(void *arg) { + pthread_cleanup_push(cleanup_handler, (void*)42); + emscripten_log(EM_LOG_CONSOLE, "Thread started!"); + pthread_mutex_lock(&mutex); + pthread_barrier_wait(&barrier); + + int ret = 0; + do { + emscripten_log(EM_LOG_CONSOLE, "Waiting on conditional variable"); + ret = pthread_cond_wait(&condvar, &mutex); + } while (ret == 0 && th_cancelled == 0); + + if (ret != 0) { + emscripten_log(EM_LOG_CONSOLE, "Cond wait failed ret: %d", ret); + } + + res = 1000; // Shouldn't ever reach here. + pthread_cleanup_pop(0); + + pthread_mutex_unlock(&mutex); + pthread_barrier_wait(&barrier); + return NULL; +} + +int main() { + if (!emscripten_has_threading_support()) { + printf("Skipped: Threading is not supported.\n"); + return 1; + } + + pthread_barrier_init(&barrier, nullptr, 2); + + pthread_t thr; + int s = pthread_create(&thr, NULL, thread_start, (void*)0); + assert(s == 0); + emscripten_log(EM_LOG_CONSOLE, "Thread created"); + + pthread_barrier_wait(&barrier); + + // Lock mutex to ensure that thread is waiting + pthread_mutex_lock(&mutex); + + emscripten_log(EM_LOG_CONSOLE, "Canceling thread.."); + s = pthread_cancel(thr); + assert(s == 0); + th_cancelled = 1; + pthread_mutex_unlock(&mutex); + + emscripten_log(EM_LOG_CONSOLE, "Main thread waitnig for side-thread"); + pthread_barrier_wait(&barrier); + pthread_barrier_destroy(&barrier); + + emscripten_log(EM_LOG_CONSOLE, "Test finished result: %d", res); + return res; +} diff --git a/tests/test_browser.py b/tests/test_browser.py index 74322a969ed3d..eae0b660d4ee9 100644 --- a/tests/test_browser.py +++ b/tests/test_browser.py @@ -3771,6 +3771,11 @@ def test_std_thread_detach(self): def test_pthread_cancel(self): self.btest(path_from_root('tests', 'pthread', 'test_pthread_cancel.cpp'), expected='1', args=['-O3', '-s', 'USE_PTHREADS', '-s', 'PTHREAD_POOL_SIZE=8']) + # Test that pthread_cancel() cancels pthread_cond_wait() operation + @requires_threads + def test_pthread_cancel_cond_wait(self): + self.btest_exit(path_from_root('tests', 'pthread', 'test_pthread_cancel_cond_wait.cpp'), expected='1', args=['-O3', '-s', 'USE_PTHREADS=1', '-s', 'PTHREAD_POOL_SIZE=8']) + # Test pthread_kill() operation @no_chrome('pthread_kill hangs chrome renderer, and keep subsequent tests from passing') @requires_threads diff --git a/tests/test_core.py b/tests/test_core.py index e569327f67077..d790778791644 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -8148,7 +8148,6 @@ def test_pthread_c11_threads(self): self.set_setting('INITIAL_MEMORY', '64mb') self.do_run_in_out_file_test('tests', 'pthread', 'test_pthread_c11_threads.c') - @no_asan('flakey errors that must be fixed, https://github.com/emscripten-core/emscripten/issues/12985') @node_pthreads def test_pthread_cxx_threads(self): self.set_setting('PROXY_TO_PTHREAD') diff --git a/tests/test_posixtest.py b/tests/test_posixtest.py index 1820eb3f9912b..6e1cc45e9c693 100644 --- a/tests/test_posixtest.py +++ b/tests/test_posixtest.py @@ -147,10 +147,7 @@ def get_pthread_tests(): **flaky, 'test_pthread_create_11_1': 'never returns', 'test_pthread_barrier_wait_2_1': 'never returns', - 'test_pthread_cond_timedwait_2_6': 'never returns', - 'test_pthread_cond_timedwait_4_3': 'never returns', 'test_pthread_attr_setscope_5_1': 'internally skipped (PTS_UNTESTED)', - 'test_pthread_cond_wait_2_3': 'never returns', 'test_pthread_create_5_1': 'never returns', 'test_pthread_exit_1_2': 'never returns', 'test_pthread_exit_2_2': 'never returns',