Use musl's pthread_detach

The musl version handles that case where pthread_detach is called
on a thread that is in the middle of exit'ing.

Author: sbc100

.circleci/config.yml

@@ -235,7 +235,10 @@ commands:
           command: |
             # Using stable rather than beta until we can fix
             # https://github.com/emscripten-core/emscripten/issues/14369
-            wget -O ~/chrome.deb https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb
+            # Currently downloading form our own buckets due to:
+            # https://github.com/emscripten-core/emscripten/issues/14987
+            #wget -O ~/chrome.deb https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb
+            wget -O ~/chrome.deb https://storage.googleapis.com/webassembly/chrome/google-chrome-stable_current_amd64.deb
             dpkg -i ~/chrome.deb
       - run:
           name: run tests

src/library_pthread.js

@@ -285,11 +285,13 @@ var LibraryPThread = {
           err('Thread ' + d['threadId'] + ': ' + d['text']);
         } else if (cmd === 'alert') {
           alert('Thread ' + d['threadId'] + ': ' + d['text']);
-        } else if (cmd === 'exit') {
-          var detached = worker.pthread && Atomics.load(HEAPU32, (worker.pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.detached }}}) >> 2);
-          if (detached) {
-            PThread.returnWorkerToPool(worker);
-          }
+        } else if (cmd === 'detachedExit') {
+#if ASSERTIONS
+          assert(worker.pthread);
+          var detached = Atomics.load(HEAPU32, (worker.pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.detached }}}) >> 2);
+          assert(detached);
+#endif
+          PThread.returnWorkerToPool(worker);
         } else if (cmd === 'exitProcess') {
           // A pthread has requested to exit the whole application process (runtime).
 #if ASSERTIONS
@@ -331,7 +333,7 @@ var LibraryPThread = {
         worker.on('error', function(e) {
           worker.onerror(e);
         });
-        worker.on('exit', function() {
+        worker.on('detachedExit', function() {
           // TODO: update the worker queue?
           // See: https://github.com/emscripten-core/emscripten/issues/9763
         });
@@ -901,29 +903,14 @@ var LibraryPThread = {
     return 0;
   },
 
-  __pthread_detach_js__sig: 'vi',
-  __pthread_detach_js: function(thread) {
-    if (!thread) {
-      err('pthread_detach attempted on a null thread pointer!');
-      return {{{ cDefine('ESRCH') }}};
-    }
-    var self = {{{ makeGetValue('thread', C_STRUCTS.pthread.self, 'i32') }}};
-    if (self !== thread) {
-      err('pthread_detach attempted on thread ' + thread + ', which does not point to a valid thread, or does not exist anymore!');
-      return {{{ cDefine('ESRCH') }}};
-    }
-    // Follow musl convention: detached:0 means not detached, 1 means the thread
-    // was created as detached, and 2 means that the thread was detached via
-    // pthread_detach.
-    var wasDetached = Atomics.compareExchange(HEAPU32, (thread + {{{ C_STRUCTS.pthread.detached }}} ) >> 2, 0, 2);
-
-    return wasDetached ? {{{ cDefine('EINVAL') }}} : 0;
-  },
-
   __pthread_exit_run_handlers__deps: ['exit'],
   __pthread_exit_run_handlers: function(status) {
     // Called from pthread_exit, either when called explicitly called
     // by programmer, or implicitly when leaving the thread main function.
+    //
+    // 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.
 
 #if PTHREADS_DEBUG
     var tb = _pthread_self();
@@ -936,13 +923,10 @@ var LibraryPThread = {
     }
   },
 
-  __pthread_exit_done: function() {
-    // Called at the end of pthread_exit, either when called explicitly called
-    // by programmer, or implicitly when leaving the thread main function.
-    //
-    // 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.
-    postMessage({ 'cmd': 'exit' });
+  __pthread_detached_exit: function() {
+    // Called at the end of pthread_exit (which occurs also when leaving the
+    // thread main function) if an only if the thread is in a detached state.
+    postMessage({ 'cmd': 'detachedExit' });
   },
 
   __cxa_thread_atexit__sig: 'vii',

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

@@ -5,6 +5,14 @@ int __pthread_join(pthread_t, void **);
 
 static int __pthread_detach(pthread_t t)
 {
+	// XXX EMSCRIPTEN: Add check for invalid (already joined) thread.  Again
+	// for the benefit of the conformance tests.
+	if (t->self != t)
+		return ESRCH;
+	// XXX EMSCRIPTEN: Even though the man page says this is undefined behaviour
+	// we have several tests in the posixtest suite that depend on this.
+	if (t->detached)
+		return EINVAL;
 	/* Cannot detach a thread that's already exiting */
 	if (a_swap(t->exitlock, 1))
 		return __pthread_join(t, 0);
@@ -15,3 +23,5 @@ static int __pthread_detach(pthread_t t)
 
 weak_alias(__pthread_detach, pthread_detach);
 weak_alias(__pthread_detach, thrd_detach);
+// XXX EMSCRIPTEN: add extra alias for asan.
+weak_alias(__pthread_detach, emscripten_builtin_pthread_detach);

system/lib/pthread/pthread_create.c

@@ -20,7 +20,7 @@ extern int __cxa_thread_atexit(void (*)(void *), void *, void *);
 extern int __pthread_create_js(struct pthread *thread, const pthread_attr_t *attr, void *(*start_routine) (void *), void *arg);
 extern void _emscripten_thread_init(int, int, int);
 extern void __pthread_exit_run_handlers();
-extern void __pthread_exit_done();
+extern void __pthread_detached_exit();
 extern int8_t __dso_handle;
 
 static void dummy_0()
@@ -97,6 +97,8 @@ void _emscripten_thread_exit(void* result) {
   // Call into the musl function that runs destructors of all thread-specific data.
   __pthread_tsd_run_dtors();
 
+  __lock(self->exitlock);
+
   if (self == emscripten_main_browser_thread_id()) {
     // FIXME(sbc): When pthread_exit causes the entire application to exit
     // we should be returning zero (according to the man page for pthread_exit).
@@ -112,13 +114,19 @@ void _emscripten_thread_exit(void* result) {
   // Not hosting a pthread anymore in this worker set __pthread_self to NULL
   _emscripten_thread_init(0, 0, 0);
 
-  // Mark the thread as no longer running.
+  // Cache deteched state since once we set threadStatus to 1, the `self` struct
+  // could be freed and reused.
+  int detatched = self->detached;
+
+  // Mark the thread as no longer running so it can be joined.
   // Once we publish this, any threads that are waiting to join with us can
-  // proceed and this worker can be recycled and used to another thread.
+  // proceed and this worker can be recycled and used on another thread.
   self->threadStatus = 1;
   emscripten_futex_wake(&self->threadStatus, INT_MAX); // wake all threads
 
-  __pthread_exit_done();
+  if (detatched) {
+    __pthread_detached_exit();
+  }
 }
 
 // Mark as `no_sanitize("address"` since emscripten_pthread_exit destroys

system/lib/pthread/pthread_detach.c

@@ -748,15 +748,14 @@ def get_files(self):
         'lock_ptc.c',
         # 'pthread_setattr_default_np.c',
         # TODO: These could be moved away from JS in the upcoming musl upgrade.
-        'pthread_cancel.c', 'pthread_detach.c',
+        'pthread_cancel.c',
         'pthread_join.c', 'pthread_testcancel.c',
       ]
       libc_files += files_in_path(
         path='system/lib/pthread',
         filenames=[
           'library_pthread.c',
           'pthread_create.c',
-          'pthread_detach.c',
           'pthread_join.c',
           'pthread_testcancel.c',
           'emscripten_proxy_main.c',