Fixing POSIX testsuite pthread test fails

This patch aims for fixing failures in pthread related POSIX test from
clone of http://posixtest.sourceforge.net/

Following problems are addressed:
1. pthread_cond_wait/2-3 test hang
  - Disabling recursive thread cancellation.
  - Allowing __timedwait_cp to be true cancellation point as _cp
    suffix suggests
2. pthread_getschedparam/1-3 test hangs sometimes:
   In pthread_barrier_wait adding check if lock is held by main thread
   and waiting on futex in small slices of time there, to check if
   there is some work to do on behalf of Worker Threads.

Signed-off-by: Adam Bujalski <[email protected]>

Author: abujalski

src/library_pthread.js

@@ -178,6 +178,24 @@ var LibraryPThread = {
       if (ENVIRONMENT_IS_PTHREAD && _pthread_self()) ___pthread_tsd_run_dtors();
     },
 
+    onThreadExit: function(tb, exitCode) {
+      // 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) {
@@ -191,19 +209,8 @@ var LibraryPThread = {
         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);
+        PThread.onThreadExit(tb, exitCode);
 
-        // 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.
         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.
@@ -213,13 +220,8 @@ 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.onThreadExit(tb, -1/*PTHREAD_CANCELED*/);
       postMessage({ 'cmd': 'cancelDone' });
     },
 

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())) {

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_runtime_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;
 	}
 

tests/pthread/test_pthread_cancel_cond_wait.cpp

@@ -0,0 +1,97 @@
+// Copyright 2020 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 <assert.h>
+#include <unistd.h>
+#include <errno.h>
+#include <emscripten.h>
+#include <emscripten/threading.h>
+
+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)
+{
+  EM_ASM(out('Called clean-up handler with arg ' + $0), arg);
+  int a = (int)arg;
+  res -= a;
+
+  pthread_mutex_unlock(&mutex);
+  pthread_barrier_wait(&barrier);
+}
+
+static void *thread_start(void *arg)
+{
+  pthread_cleanup_push(cleanup_handler, (void*)42);
+  EM_ASM(out('Thread started!'));
+  pthread_mutex_lock(&mutex);
+  pthread_barrier_wait(&barrier);
+
+  int ret = 0;
+  do {
+    EM_ASM(out('Waiting on conditional variable'););
+    ret = pthread_cond_wait(&condvar, &mutex);
+  } while (ret == 0 && th_cancelled == 0);
+
+  if (ret != 0) {
+    EM_ASM(out('Cond wait failed ret: ' +  $0);, ret);
+  }
+
+  res = 1000; // Shouldn't ever reach here.
+  pthread_cleanup_pop(0);
+
+  pthread_mutex_unlock(&mutex);
+  pthread_barrier_wait(&barrier);
+  return NULL;
+}
+
+pthread_t thr;
+
+int main()
+{
+  if (!emscripten_has_threading_support())
+  {
+#ifdef REPORT_RESULT
+    REPORT_RESULT(1);
+#endif
+    printf("Skipped: Threading is not supported.\n");
+    return 0;
+  }
+
+  pthread_barrier_init(&barrier, nullptr, 2);
+
+  int s = pthread_create(&thr, NULL, thread_start, (void*)0);
+  assert(s == 0);
+  EM_ASM(out('Thread created'););
+
+  pthread_barrier_wait(&barrier);
+
+  // Lock mutex to ensure that thread is waiting
+  pthread_mutex_lock(&mutex);
+
+  EM_ASM(out('Canceling thread..'););
+  s = pthread_cancel(thr);
+  assert(s == 0);
+  th_cancelled = 1;
+  pthread_mutex_unlock(&mutex);
+
+  EM_ASM(out('Main thread waitnig for side-thread'););
+  pthread_barrier_wait(&barrier);
+  pthread_barrier_destroy(&barrier);
+
+  EM_ASM(out('Test finished result: ' + $0);, res);
+#ifdef REPORT_RESULT
+  REPORT_RESULT(res);
+#endif
+  return res != 1;
+}

tests/runner.py

@@ -1148,6 +1148,7 @@ def end_headers(self):
       self.send_header('Cross-Origin-Embedder-Policy', 'require-corp')
       self.send_header('Cross-Origin-Resource-Policy', 'cross-origin')
       self.send_header('Cache-Control', 'no-cache, no-store, must-revalidate')
+
       return SimpleHTTPRequestHandler.end_headers(self)
 
     def do_GET(self):

tests/test_browser.py

@@ -3756,6 +3756,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(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

tests/test_posixtest.py

@@ -140,10 +140,7 @@ def get_pthread_tests():
   **unsupported,
   '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',