From 1317cc68f01868c777c515e8c081e3e71a346bd5 Mon Sep 17 00:00:00 2001
From: Quinton Miller <nicetas.c@gmail.com>
Date: Fri, 27 Sep 2024 22:55:52 +0800
Subject: [PATCH] Fix race condition in `pthread_create` handle initialization

---
 spec/std/thread/condition_variable_spec.cr |  8 --------
 spec/std/thread/mutex_spec.cr              |  8 --------
 spec/std/thread_spec.cr                    |  8 --------
 src/crystal/system/unix/pthread.cr         | 23 ++++++++++++++++++----
 4 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/spec/std/thread/condition_variable_spec.cr b/spec/std/thread/condition_variable_spec.cr
index ff9c44204bb6..1bf78f797357 100644
--- a/spec/std/thread/condition_variable_spec.cr
+++ b/spec/std/thread/condition_variable_spec.cr
@@ -1,11 +1,3 @@
-{% if flag?(:musl) %}
-  # FIXME: These thread specs occasionally fail on musl/alpine based ci, so
-  # they're disabled for now to reduce noise.
-  # See https://github.com/crystal-lang/crystal/issues/8738
-  pending Thread::ConditionVariable
-  {% skip_file %}
-{% end %}
-
 require "../spec_helper"
 
 # interpreter doesn't support threads yet (#14287)
diff --git a/spec/std/thread/mutex_spec.cr b/spec/std/thread/mutex_spec.cr
index ff298f318329..99f3c5d385c3 100644
--- a/spec/std/thread/mutex_spec.cr
+++ b/spec/std/thread/mutex_spec.cr
@@ -1,11 +1,3 @@
-{% if flag?(:musl) %}
-  # FIXME: These thread specs occasionally fail on musl/alpine based ci, so
-  # they're disabled for now to reduce noise.
-  # See https://github.com/crystal-lang/crystal/issues/8738
-  pending Thread::Mutex
-  {% skip_file %}
-{% end %}
-
 require "../spec_helper"
 
 # interpreter doesn't support threads yet (#14287)
diff --git a/spec/std/thread_spec.cr b/spec/std/thread_spec.cr
index feb55454b621..5a43c7e429d1 100644
--- a/spec/std/thread_spec.cr
+++ b/spec/std/thread_spec.cr
@@ -1,13 +1,5 @@
 require "./spec_helper"
 
-{% if flag?(:musl) %}
-  # FIXME: These thread specs occasionally fail on musl/alpine based ci, so
-  # they're disabled for now to reduce noise.
-  # See https://github.com/crystal-lang/crystal/issues/8738
-  pending Thread
-  {% skip_file %}
-{% end %}
-
 # interpreter doesn't support threads yet (#14287)
 pending_interpreted describe: Thread do
   it "allows passing an argumentless fun to execute" do
diff --git a/src/crystal/system/unix/pthread.cr b/src/crystal/system/unix/pthread.cr
index b55839ff2784..952843f4c2b0 100644
--- a/src/crystal/system/unix/pthread.cr
+++ b/src/crystal/system/unix/pthread.cr
@@ -9,20 +9,35 @@ module Crystal::System::Thread
     @system_handle
   end
 
+  protected setter system_handle
+
   private def init_handle
-    # NOTE: the thread may start before `pthread_create` returns, so
-    # `@system_handle` must be set as soon as possible; we cannot use a separate
-    # handle and assign it to `@system_handle`, which would have been too late
+    # NOTE: `@system_handle` needs to be set here too, not just in
+    # `.thread_proc`, since the current thread might progress first; the value
+    # of `LibC.pthread_self` inside the new thread must be equal to this
+    # `@system_handle` after `pthread_create` returns
     ret = GC.pthread_create(
       thread: pointerof(@system_handle),
       attr: Pointer(LibC::PthreadAttrT).null,
-      start: ->(data : Void*) { data.as(::Thread).start; Pointer(Void).null },
+      start: ->Thread.thread_proc(Void*),
       arg: self.as(Void*),
     )
 
     raise RuntimeError.from_os_error("pthread_create", Errno.new(ret)) unless ret == 0
   end
 
+  def self.thread_proc(data : Void*) : Void*
+    th = data.as(::Thread)
+
+    # `#start` calls `#stack_address`, which might read `@system_handle` before
+    # `GC.pthread_create` updates it in the original thread that spawned the
+    # current one, so we also assign to it here
+    th.system_handle = current_handle
+
+    th.start
+    Pointer(Void).null
+  end
+
   def self.current_handle : Handle
     LibC.pthread_self
   end