Simplify thread clean / exit / terminate logic. NFC

Mostly inspired by musl.

Author: kleisauke

src/library_pthread.js

@@ -29,7 +29,6 @@
 var LibraryPThread = {
   $PThread__postset: 'PThread.init();',
   $PThread__deps: ['_emscripten_thread_init',
-                   '$killThread',
                    '$cancelThread', '$cleanupThread', '$zeroMemory',
                    '$spawnThread',
                    '_emscripten_thread_free_data',
@@ -158,10 +157,7 @@ var LibraryPThread = {
       err('terminateAllThreads');
 #endif
       for (var t in PThread.pthreads) {
-        var pthread = PThread.pthreads[t];
-        if (pthread && pthread.worker) {
-          PThread.returnWorkerToPool(pthread.worker);
-        }
+        cleanupThread(t, true);
       }
 
 #if ASSERTIONS
@@ -181,7 +177,7 @@ var LibraryPThread = {
       }
       PThread.unusedWorkers = [];
     },
-    returnWorkerToPool: function(worker) {
+    returnWorkerToPool: function(worker, terminate) {
       // We don't want to run main thread queued calls here, since we are doing
       // some operations that leave the worker queue in an invalid state until
       // we are completely done (it would be bad if free() ends up calling a
@@ -190,9 +186,13 @@ var LibraryPThread = {
       // we are all done.
       var pthread_ptr = worker.pthread.threadInfoStruct;
       delete PThread.pthreads[pthread_ptr];
-      // Note: worker is intentionally not terminated so the pool can
-      // dynamically grow.
-      PThread.unusedWorkers.push(worker);
+      if (terminate) {
+        worker.terminate();
+      } else {
+        // No termination needed? Return it to the worker pool as an
+        // unused worker so the pool can dynamically grow.
+        PThread.unusedWorkers.push(worker);
+      }
       PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(worker), 1);
       // Not a running Worker anymore
       // Detach the worker from the pthread object, and return it to the
@@ -256,9 +256,7 @@ var LibraryPThread = {
         } else if (cmd === 'spawnThread') {
           spawnThread(d);
         } else if (cmd === 'cleanupThread') {
-          cleanupThread(d['thread']);
-        } else if (cmd === 'killThread') {
-          killThread(d['thread']);
+          cleanupThread(d['thread'], d['terminate']);
         } else if (cmd === 'cancelThread') {
           cancelThread(d['thread']);
         } else if (cmd === 'loaded') {
@@ -435,44 +433,24 @@ var LibraryPThread = {
     }
   },
 
-  $killThread__deps: ['_emscripten_thread_free_data'],
-  $killThread: function(pthread_ptr) {
-#if PTHREADS_DEBUG
-    err('killThread ' + ptrToString(pthread_ptr));
-#endif
-#if ASSERTIONS
-    assert(!ENVIRONMENT_IS_PTHREAD, 'Internal Error! killThread() can only ever be called from main application thread!');
-    assert(pthread_ptr, 'Internal Error! Null pthread_ptr in killThread!');
-#endif
-    var pthread = PThread.pthreads[pthread_ptr];
-    delete PThread.pthreads[pthread_ptr];
-    pthread.worker.terminate();
-    __emscripten_thread_free_data(pthread_ptr);
-    // 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) {
     // Called when a thread needs to be cleaned up so it can be reused.
     // A thread is considered reusable when it either returns from its
     // entry point, calls pthread_exit, or acts upon a cancellation.
     // Detached threads are responsible for calling this themselves,
     // otherwise pthread_join is responsible for calling this.
-    if (!ENVIRONMENT_IS_PTHREAD) cleanupThread(thread);
-    else postMessage({ 'cmd': 'cleanupThread', 'thread': thread });
+    if (!ENVIRONMENT_IS_PTHREAD) cleanupThread(thread, false);
+    else postMessage({ 'cmd': 'cleanupThread', 'thread': thread, 'terminate': false });
   },
 
-  $cleanupThread: function(pthread_ptr) {
+  $cleanupThread: function(pthread_ptr, terminate) {
 #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];
     assert(pthread);
-    var worker = pthread.worker;
-    PThread.returnWorkerToPool(worker);
+    PThread.returnWorkerToPool(pthread.worker, terminate);
   },
 
 #if MAIN_MODULE
@@ -799,8 +777,8 @@ var LibraryPThread = {
       if (!ENVIRONMENT_IS_PTHREAD) cancelThread(thread);
       else postMessage({ 'cmd': 'cancelThread', 'thread': thread });
     } else {
-      if (!ENVIRONMENT_IS_PTHREAD) killThread(thread);
-      else postMessage({ 'cmd': 'killThread', 'thread': thread });
+      if (!ENVIRONMENT_IS_PTHREAD) cleanupThread(thread, true);
+      else postMessage({ 'cmd': 'cleanupThread', 'thread': thread, 'terminate': true });
     }
     return 0;
   },

system/lib/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

@@ -557,7 +557,7 @@ tid_t GetTid() {
 #elif SANITIZER_SOLARIS
   return thr_self();
 #elif SANITIZER_EMSCRIPTEN
-  return (tid_t) pthread_self();
+  return gettid();
 #else
   return internal_syscall(SYSCALL(gettid));
 #endif

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

@@ -6,8 +6,7 @@ static int __pthread_detach(pthread_t t)
 #ifdef __EMSCRIPTEN__
 	// XXX EMSCRIPTEN: Add check for invalid (already joined) thread.  Again
 	// for the benefit of the conformance tests.
-	if (!_emscripten_thread_is_valid(t))
-		return ESRCH;
+	if (!_emscripten_thread_is_valid(t)) return ESRCH;
 #endif
 	/* If the cas fails, detach state is either already-detached
 	 * or exiting/exited, and pthread_join will trap or cleanup. */

system/lib/pthread/library_pthread.c

@@ -639,7 +639,7 @@ int emscripten_dispatch_to_thread_async_(pthread_t target_thread,
 }
 
 int _emscripten_thread_is_valid(pthread_t thread) {
-  return thread->self == thread;
+  return thread->tid;
 }
 
 static void *dummy_tsd[1] = { 0 };
@@ -650,8 +650,7 @@ __attribute__((constructor(48)))
 void __emscripten_init_main_thread(void) {
   __emscripten_init_main_thread_js(&__main_pthread);
 
-  // The pthread struct has a field that points to itself - this is used as
-  // a magic ID to detect whether the pthread_t structure is 'alive'.
+  // The pthread struct has a field that points to itself.
   __main_pthread.self = &__main_pthread;
   __main_pthread.stack = (void*)emscripten_stack_get_base();
   __main_pthread.stack_size = emscripten_stack_get_base() - emscripten_stack_get_end();

system/lib/pthread/pthread_create.c

@@ -135,9 +135,10 @@ int __pthread_create(pthread_t* restrict res,
   new->map_base = block;
   new->map_size = size;
 
-  // The pthread struct has a field that points to itself - this is used as a
-  // magic ID to detect whether the pthread_t structure is 'alive'.
+  // The pthread struct has a field that points to itself.
   new->self = new;
+
+  // Thread ID, this becomes zero when the thread is no longer available.
   new->tid = next_tid++;
 
   // pthread struct robust_list head should point to itself.
@@ -226,20 +227,23 @@ void _emscripten_thread_free_data(pthread_t t) {
   // A thread can never free its own thread data.
   assert(t != pthread_self());
 #ifndef NDEBUG
-  if (t->profilerBlock) {
-    emscripten_builtin_free(t->profilerBlock);
+  intptr_t old_block = __c11_atomic_exchange((_Atomic intptr_t*)&t->profilerBlock, 0, __ATOMIC_SEQ_CST);
+  if (old_block) {
+    emscripten_builtin_free((void*)old_block);
   }
 #endif
+  // The tid may be reused.  Clear it to prevent inadvertent use
+  // and inform functions that would use it that it's no longer
+  // available.
+  t->tid = 0;
 
-  // Free all the enture thread block (called map_base because
+  // Free the entire thread block (called map_base because
   // musl normally allocates this using mmap).  This region
   // includes the pthread structure itself.
   unsigned char* block = t->map_base;
 #ifdef PTHREAD_DEBUG
   _emscripten_errf("_emscripten_thread_free_data thread=%p map_base=%p", t, block);
 #endif
-  // To aid in debugging, set the entire region to zero.
-  memset(block, 0, sizeof(struct pthread));
   emscripten_builtin_free(block);
 }