Fix races around pthread exit and join (#409)

Author: yamt

Makefile

@@ -325,6 +325,7 @@ ifeq ($(THREAD_MODEL), posix)
 # Specify the tls-model until LLVM 15 is released (which should contain
 # https://reviews.llvm.org/D130053).
 CFLAGS += -mthread-model posix -pthread -ftls-model=local-exec
+ASMFLAGS += -matomics
 
 # Include cloudlib's directory to access the structure definition of clockid_t
 CFLAGS += -I$(LIBC_BOTTOM_HALF_CLOUDLIBC_SRC)

libc-top-half/musl/src/thread/pthread_create.c

@@ -164,14 +164,6 @@ static void __pthread_exit(void *result)
 	self->prev->next = self->next;
 	self->prev = self->next = self;
 
-#ifndef __wasilibc_unmodified_upstream
-	/* On Linux, the thread is created with CLONE_CHILD_CLEARTID,
-	 * and this lock will unlock by kernel when this thread terminates.
-	 * So we should unlock it here in WebAssembly.
-	 * See also set_tid_address(2) */
-	__tl_unlock();
-#endif
-
 #ifdef __wasilibc_unmodified_upstream
 	if (state==DT_DETACHED && self->map_base) {
 		/* Detached threads must block even implementation-internal
@@ -190,9 +182,6 @@ static void __pthread_exit(void *result)
 	}
 #else
 	if (state==DT_DETACHED && self->map_base) {
-		// __syscall(SYS_exit) would unlock the thread, list
-		// do it manually here
-		__tl_unlock();
 		free(self->map_base);
 		// Can't use `exit()` here, because it is too high level
 		return;
@@ -212,10 +201,15 @@ static void __pthread_exit(void *result)
 #ifdef __wasilibc_unmodified_upstream
 	for (;;) __syscall(SYS_exit, 0);
 #else
-	// __syscall(SYS_exit) would unlock the thread, list
-	// do it manually here
-	__tl_unlock();
 	// Can't use `exit()` here, because it is too high level
+
+	/* On Linux, the thread is created with CLONE_CHILD_CLEARTID,
+	 * and the lock (__thread_list_lock) will be unlocked by kernel when
+	 * this thread terminates.
+	 * See also set_tid_address(2)
+	 *
+	 * In WebAssembly, we leave it to wasi_thread_start instead.
+	 */
 #endif
 }
 

libc-top-half/musl/src/thread/wasm32/wasi_thread_start.s

@@ -28,4 +28,21 @@ wasi_thread_start:
 	local.get   1  # start_arg
 	call __wasi_thread_start_C
 
+	# Unlock thread list. (as CLONE_CHILD_CLEARTID would do for Linux)
+	#
+	# Note: once we unlock the thread list, our "map_base" can be freed
+	# by a joining thread. It's safe as we are in ASM and no longer use
+	# our C stack or pthread_t. It's impossible to do this safely in C
+	# because there is no way to tell the C compiler not to use C stack.
+	i32.const   __thread_list_lock
+	i32.const   0
+	i32.atomic.store 0
+	# As an optimization, we can check tl_lock_waiters here.
+	# But for now, simply wake up unconditionally as
+	# CLONE_CHILD_CLEARTID does.
+	i32.const   __thread_list_lock
+	i32.const   1
+	memory.atomic.notify 0
+	drop
+
 	end_function