Add locks, with some minor improvements to threads

1 files changed, 27 insertions, 21 deletions

diff --git a/lib/thread_finish.asm b/lib/thread_finish.asm
index 9392f1c..860b0a4 100644
--- a/lib/thread_finish.asm
+++ b/lib/thread_finish.asm
@@ -3,9 +3,9 @@
 
 ; Cheat sheet for Linux' x86_64 calling convention:
 ;
-; - free to overwrite; the caller should save them:
+; - free to overwrite (caller should save them):
 ;       rax, rcx, rdx, rsi, rdi, r8-r11, xmm0-xmm15
-; - caller expects no change; callee should save them:
+; - caller expects be kept (callee should save them):
 ;       rbx, rbp, r12-r15
 ;
 ; - for passing paramters to functions:
@@ -28,8 +28,8 @@ section .text
 %define SYS_MUNMAP  11
 %define SYS_FUTEX  202
 
-; Relevant flags for futex
-%define FUTEX_WAIT         0x00
+; Relevant operations for futex
+%define FUTEX_WAIT         0
 %define FUTEX_PRIVATE_FLAG 0x80
 
 
@@ -38,8 +38,8 @@ section .text
 
 
 ; Wait for thread to exit, save its return value, and clean up. Arguments:
-;	rdi: u32*   = handle of the thread to wait for
-;	rsi: void** = where to put the void* returned by the thread
+;	rdi: struct{u32,u32}* = handle of the thread to wait for
+;	rsi: void**           = where to put void* returned by thread
 ; Returns zero on success, or a standard error code.
 global linen_thread_finish
 linen_thread_finish:
@@ -51,15 +51,15 @@ linen_thread_finish:
 		mov eax, -22 ; (EINVAL = -22)
 
 		test rdi, rdi
-		jz join_end ; rdi is NULL
+		jz finish_return ; rdi is NULL
 
-		; rdi is nonzero, so let's just assume it's a valid pointer.
-		; If that assumption is wrong we'll get a segmentation fault.
+		; rdi is nonzero, so let's just assume it's a valid pointer;
+		; if that assumption is wrong we'll get a segmentation fault.
 		; But we don't yet trust that [rdi] is a valid thread handle!
-		; To verify this, we check the canary value stored at [rdi + 4].
+		; To verify this we check the canary value stored at [rdi + 4].
 		mov ecx, [rdi + 4]
 		cmp ecx, 0xDEADBEEF
-		jnz join_end
+		jnz finish_return
 
 		;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 		;;;; Wait until thread is finished ;;;;
@@ -68,15 +68,16 @@ linen_thread_finish:
 		; We'll clobber rsi if we need to set up a futex call
 		mov r8, rsi
 
+	finish_retry:
 		; When spawning, we set CLONE_CHILD_SETTID and CLONE_CHILD_CLEARTID:
 		; [rdi] contains the child thread's TID, and will get automatically
 		; cleared (to 0) when the child exits; this is what we'll watch for.
 
 		; Atomically check whether the target thread is still running.
-		; if ([rdi] == 0) { goto join_done; } else { eax = [rdi]; }
+		; if ([rdi] == 0) { goto finish_success; } else { eax = [rdi]; }
 		xor eax, eax
 		lock cmpxchg [rdi], eax
-		jz join_done
+		jz finish_success
 
 		; The thread is still busy, so block until it's done.
 		; The futex system call waits until the dword at an
@@ -84,7 +85,7 @@ linen_thread_finish:
 		; See: man 2 futex
 
 		; futex: rdi = uaddr: address of the dword to watch
-		; futex: rsi = futex_op: which futex operation we want
+		; futex: rsi = futex_op: which futex operation we want:
 		; - FUTEX_WAIT:         block until the value at [rdi] changes
 		; - FUTEX_PRIVATE_FLAG: FIXME waits forever, I don't understand why
 		mov esi, FUTEX_WAIT
@@ -93,21 +94,26 @@ linen_thread_finish:
 		; futex: r10 = timeout: in case we had a deadline (we don't)
 		xor r10, r10
 		; futex: r8 = uaddr2: ignored when FUTEX_WAIT is used
-		; futex: r9 = val3:   ignored when FUTEX WAIT is used
+		; futex: r9 = val3:   ignored when FUTEX_WAIT is used
 		; futex: rax = system call ID
 		mov eax, SYS_FUTEX
 		; futex: rax = futex(rdi, rsi, rdx, r10, (r8), (r9))
 		syscall
 
-		; Check result of futex: negative means failure
+		; Sometimes the thread exits after the "lock cmpxchg" instruction
+		; but before the futex call. In that case, futex returns EAGAIN.
+		cmp rax, -11 ; (EAGAIN = -11)
+		je finish_retry
+
+		; Any other nonzero return value means failure
 		test rax, rax
-		jnz join_end
+		jnz finish_return
 
 		;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 		;;;; Clean up after thread's exit ;;;;
 		;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
-	join_done:
+	finish_success:
 		; The thread left its function return value on the stack, read it
 		mov rdx, [rdi - 8]
 
@@ -127,15 +133,15 @@ linen_thread_finish:
 
 		; Check result of munmap: nonzero means failure
 		test rax, rax
-		jnz join_end
+		jnz finish_return
 
 		; Check if caller gave a location (r8) to save the return value (rdx)
 		test r8, r8
-		jz join_end ; caller doesn't care: gave NULL pointer
+		jz finish_return ; caller doesn't care: gave NULL pointer
 		mov [r8], rdx
 		; Note: if munmap failed, the buffer is still there, so we
 		; can safely return an error without losing the return value.
 
-	join_end:
+	finish_return:
 		ret