diff --git a/system/lib/libc/musl/src/thread/__timedwait.c b/system/lib/libc/musl/src/thread/__timedwait.c index 9539b0b90c1a3..6da7dad3faaa4 100644 --- a/system/lib/libc/musl/src/thread/__timedwait.c +++ b/system/lib/libc/musl/src/thread/__timedwait.c @@ -61,8 +61,8 @@ int __timedwait_cp(volatile int *addr, int val, double msecsToSleep = top ? (top->tv_sec * 1000 + top->tv_nsec / 1000000.0) : INFINITY; // 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. + // by the user, unless current thread has cancelation disabled (which may be either done + // directly, or indirectly, for example in the __timedwait() function). pthread_t self = pthread_self(); if (self->canceldisable != PTHREAD_CANCEL_DISABLE) { @@ -75,15 +75,14 @@ int __timedwait_cp(volatile int *addr, int val, // __pthread_testcancel(), which won't return at all. __pthread_testcancel(); // If __pthread_testcancel does return here it means that canceldisable - // must be set to PTHREAD_CANCEL_MASKED. This appears to mean "return - // ECANCELLED to the caller". See pthread_cond_timedwait.c for the only - // use of this that I could find. + // must be set to PTHREAD_CANCEL_MASKED. In this case we emulate the + // behaviour of the futex syscall and return ECANCELLED here. + // See pthread_cond_timedwait.c for the only use of this flag. return ECANCELED; } msecsToSleep = sleepUntilTime - emscripten_get_now(); if (msecsToSleep <= 0) { - r = ETIMEDOUT; - break; + return ETIMEDOUT; } // Must wait in slices in case this thread is cancelled in between. if (msecsToSleep > max_ms_slice_to_sleep) diff --git a/system/lib/libc/musl/src/thread/pthread_cancel.c b/system/lib/libc/musl/src/thread/pthread_cancel.c index 91c779ed6e657..3428cd5fab474 100644 --- a/system/lib/libc/musl/src/thread/pthread_cancel.c +++ b/system/lib/libc/musl/src/thread/pthread_cancel.c @@ -8,13 +8,7 @@ hidden long __cancel(), __syscall_cp_asm(), __syscall_cp_c(); long __cancel() { pthread_t self = __pthread_self(); -#ifdef __EMSCRIPTEN__ - // Emscripten doesn't have actual async cancelation so we make a best effort - // by cancelling cooperatively when self->cancelasync is set. - if (self->canceldisable == PTHREAD_CANCEL_ENABLE || self->cancelasync) -#else if (self->canceldisable == PTHREAD_CANCEL_ENABLE) -#endif pthread_exit(PTHREAD_CANCELED); self->canceldisable = PTHREAD_CANCEL_DISABLE; return -ECANCELED; @@ -83,12 +77,7 @@ static void cancel_handler(int sig, siginfo_t *si, void *ctx) void __testcancel() { pthread_t self = __pthread_self(); -#ifdef __EMSCRIPTEN__ - // See comment above about cancelasync under emscripten. - if (self->cancel && (self->cancelasync || !self->canceldisable)) -#else if (self->cancel && !self->canceldisable) -#endif __cancel(); } diff --git a/system/lib/pthread/emscripten_futex_wait.c b/system/lib/pthread/emscripten_futex_wait.c index 7a37e9bc33fcc..b74fd459634fb 100644 --- a/system/lib/pthread/emscripten_futex_wait.c +++ b/system/lib/pthread/emscripten_futex_wait.c @@ -132,7 +132,7 @@ int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms) #ifdef __EMSCRIPTEN_PTHREADS__ pthread_t self = pthread_self(); - bool cancelable = self->cancelasync; + bool cancelable = !self->canceldisable && self->cancelasync; #else bool cancelable = false; #endif diff --git a/test/codesize/test_codesize_minimal_pthreads.json b/test/codesize/test_codesize_minimal_pthreads.json index 286657e60f5b4..d78c1fce61512 100644 --- a/test/codesize/test_codesize_minimal_pthreads.json +++ b/test/codesize/test_codesize_minimal_pthreads.json @@ -1,10 +1,10 @@ { "a.out.js": 7363, "a.out.js.gz": 3604, - "a.out.nodebug.wasm": 19239, - "a.out.nodebug.wasm.gz": 8916, - "total": 26602, - "total_gz": 12520, + "a.out.nodebug.wasm": 19244, + "a.out.nodebug.wasm.gz": 8919, + "total": 26607, + "total_gz": 12523, "sent": [ "a (memory)", "b (emscripten_get_now)", diff --git a/test/codesize/test_codesize_minimal_pthreads_memgrowth.json b/test/codesize/test_codesize_minimal_pthreads_memgrowth.json index 2136612149cb0..65f9b3f7a2d7b 100644 --- a/test/codesize/test_codesize_minimal_pthreads_memgrowth.json +++ b/test/codesize/test_codesize_minimal_pthreads_memgrowth.json @@ -1,10 +1,10 @@ { "a.out.js": 7765, "a.out.js.gz": 3810, - "a.out.nodebug.wasm": 19240, - "a.out.nodebug.wasm.gz": 8917, - "total": 27005, - "total_gz": 12727, + "a.out.nodebug.wasm": 19245, + "a.out.nodebug.wasm.gz": 8920, + "total": 27010, + "total_gz": 12730, "sent": [ "a (memory)", "b (emscripten_get_now)", diff --git a/test/pthread/test_pthread_cancel_async.c b/test/pthread/test_pthread_cancel_async.c index 3c7ac63915db8..5897f1f6e8061 100644 --- a/test/pthread/test_pthread_cancel_async.c +++ b/test/pthread/test_pthread_cancel_async.c @@ -26,6 +26,7 @@ void emscripten_outf(const char* msg, ...) { pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; _Atomic bool started = false; +_Atomic bool timedlock_returned = false; _Atomic bool done_cleanup = false; void cleanup_handler(void *arg) { @@ -47,12 +48,20 @@ void *thread_start(void *arg) { // Signal the main thread that are started started = true; - // This mutex is locked by the main thread so this call should never return. - // pthread_mutex_lock is not a cancellation point so deferred cancellation - // won't work here, async cancellation should. - pthread_mutex_lock(&mutex); + // At least under musl, async cancellation also does not work for + // pthread_mutex_lock so this call to pthread_mutex_timedlock should always + // timeout. + struct timespec ts; + clock_gettime(CLOCK_REALTIME, &ts); + ts.tv_sec += 2; + int rc = pthread_mutex_timedlock(&mutex, &ts); + timedlock_returned = true; + assert(rc == ETIMEDOUT); + emscripten_out("pthread_mutex_timedlock timed out"); + + pthread_testcancel(); - assert(false && "pthread_mutex_lock returned!"); + assert(false && "pthread_testcancel returned!"); pthread_cleanup_pop(0); } @@ -79,6 +88,7 @@ int main() { emscripten_out("Joining thread.."); s = pthread_join(thr, NULL); assert(s == 0); + assert(timedlock_returned); emscripten_out("done"); return 0; } diff --git a/test/pthread/test_pthread_cancel_async.out b/test/pthread/test_pthread_cancel_async.out index 53e6afcae01bb..40e0468523272 100644 --- a/test/pthread/test_pthread_cancel_async.out +++ b/test/pthread/test_pthread_cancel_async.out @@ -1,6 +1,7 @@ Starting thread.. Thread started! Canceling thread.. +pthread_mutex_timedlock timed out Called clean-up handler with arg 42 Joining thread.. done diff --git a/test/test_posixtest.py b/test/test_posixtest.py index 1d57ea9f58d59..d696ebdcc4cfd 100644 --- a/test/test_posixtest.py +++ b/test/test_posixtest.py @@ -77,6 +77,7 @@ def get_tests(): 'test_pthread_join_6_3': 'creates too many threads', 'test_pthread_barrier_wait_3_2': 'signals are not supported', 'test_pthread_cond_broadcast_1_2': 'tries to create 10,0000 threads, then depends on fork()', + 'test_pthread_setcanceltype_1_1': 'async cancelation does not work withing pthread_mutex_lock', } unsupported = {