Fix for async cancellation during __timedwait. NFC (#19963)

Emscripten doesn't have real async cancellation but we don't want
__timedwait_cp to return the thread has been canceled while while its
running.

This fixes a problem in the test_pthread_setcanceltype_1_1 test which
was not being correctly reported but shows up as a real problem with
the switch to the new proxying system in #19947.

Specifically, Without this change, a thread this async canceled during
pthread_mutex_lock call will return ECANCELED, but instead it should
never return and thread should be canceled, which is the behaviour
after this change.

Author: sbc100

system/lib/libc/musl/src/thread/__timedwait.c

@@ -69,13 +69,18 @@ int __timedwait_cp(volatile int *addr, int val,
 	// which may be either done by the user of __timedwait() function.
 	if (is_runtime_thread ||
 	    pthread_self()->canceldisable != PTHREAD_CANCEL_DISABLE ||
-	    pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) {
+	    pthread_self()->cancelasync) {
 		double sleepUntilTime = emscripten_get_now() + msecsToSleep;
 		do {
 			if (pthread_self()->cancel) {
-				// Emscripten-specific return value: The wait was canceled by user calling
-				// pthread_cancel() for this thread, and the caller needs to cooperatively
-				// cancel execution.
+				// The thread was canceled by pthread_cancel().
+				// In the case of cancelasync or PTHREAD_CANCEL_ENABLE we can just call
+				// __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 appear to mean "return
+				// ECANCELLED to the caller".  See pthread_cond_timedwait.c for the only
+				// use of this that I could find.
 				return ECANCELED;
 			}
 			msecsToSleep = sleepUntilTime - emscripten_get_now();

system/lib/libc/musl/src/thread/pthread_cancel.c

@@ -8,7 +8,13 @@ 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;
@@ -77,7 +83,12 @@ 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();
 }
 

test/pthread/test_pthread_cancel.out

@@ -0,0 +1,4 @@
+Canceling thread..
+Thread started!
+Called clean-up handler with arg 42
+After canceling, shared variable = 1.

test/pthread/test_pthread_cancel_async.c

@@ -0,0 +1,73 @@
+// Copyright 2015 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.h>
+#include <sys/types.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <unistd.h>
+#include <errno.h>
+#include <emscripten/console.h>
+
+pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+_Atomic long res = 43;
+_Atomic int started = false;
+
+static void cleanup_handler(void *arg)
+{
+  long a = (long)arg;
+  emscripten_outf("Called clean-up handler with arg %ld", a);
+  res -= a;
+}
+
+static void *thread_start(void *arg) {
+  // Setup thread for async cancelation only
+  pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
+  pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
+
+  pthread_cleanup_push(cleanup_handler, (void*)42);
+
+  emscripten_out("Thread started!");
+
+  // 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 cancelation should.
+  pthread_mutex_lock(&mutex);
+
+  assert(false && "pthread_mutex_lock returned!");
+  pthread_cleanup_pop(0);
+}
+
+int main() {
+  pthread_mutex_lock(&mutex);
+
+  emscripten_out("Starting thread..");
+  pthread_t thr;
+  int s = pthread_create(&thr, NULL, thread_start, (void*)0);
+  assert(s == 0);
+  // Busy wait until thread is started
+  while (!started) {
+    sched_yield();
+  }
+
+  emscripten_out("Canceling thread..");
+  s = pthread_cancel(thr);
+  assert(s == 0);
+  // Busy wait until thread cancel handler has been run
+  while (res != 1) {
+    sched_yield();
+  }
+
+  emscripten_out("Joining thread..");
+  s = pthread_join(thr, NULL);
+  assert(s == 0);
+  emscripten_out("done");
+  return 0;
+}

test/pthread/test_pthread_cancel_async.out

@@ -0,0 +1,6 @@
+Starting thread..
+Thread started!
+Canceling thread..
+Called clean-up handler with arg 42
+Joining thread..
+done

test/test_core.py

@@ -2691,6 +2691,14 @@ def test_atexit_threads(self):
     self.set_setting('EXIT_RUNTIME')
     self.do_core_test('test_atexit_threads.cpp')
 
+  @node_pthreads
+  def test_pthread_cancel(self):
+    self.do_run_in_out_file_test('pthread/test_pthread_cancel.cpp')
+
+  @node_pthreads
+  def test_pthread_cancel_async(self):
+    self.do_run_in_out_file_test('pthread/test_pthread_cancel_async.c')
+
   @no_asan('test relies on null pointer reads')
   def test_pthread_specific(self):
     self.do_run_in_out_file_test('pthread/specific.c')