From b52c19f81396135a74d8a950a46d5f9ba3381a0f Mon Sep 17 00:00:00 2001
From: Julien Portalier <julien@portalier.com>
Date: Mon, 6 May 2024 10:20:44 +0200
Subject: [PATCH] fixup! Fix: GC safety of Thread#start

---
 src/crystal/system/thread.cr | 14 ++++++++------
 src/gc/boehm.cr              | 11 ++++++++---
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/crystal/system/thread.cr b/src/crystal/system/thread.cr
index fb249171a078..a762e31312b8 100644
--- a/src/crystal/system/thread.cr
+++ b/src/crystal/system/thread.cr
@@ -140,14 +140,16 @@ class Thread
     rescue ex
       @exception = ex
     ensure
-      # to shutdown the thread we first call any function that needs to access
-      # local or instance variables, then start cleaning up references (that
-      # may be the last reference, so the GC may collect them anytime):
-      detach { system_close }
-      Fiber.inactive(fiber)
+      {% if flag?(:preview_mt) %}
+        # fix the thread stack now so we can start cleaning up references
+        GC.lock_read
+        GC.set_stackbottom(thread, fiber.@stack_bottom)
+        GC.unlock_read
+      {% end %}
 
-      # eventually forget the thread reference (must be the last thing to do)
       Thread.threads.delete(self)
+      Fiber.inactive(fiber)
+      detach { system_close }
     end
   end
 
diff --git a/src/gc/boehm.cr b/src/gc/boehm.cr
index 29ae825adab1..ba4b34734089 100644
--- a/src/gc/boehm.cr
+++ b/src/gc/boehm.cr
@@ -298,10 +298,10 @@ module GC
 
   # :nodoc:
   {% if flag?(:preview_mt) %}
-    def self.set_stackbottom(thread_handle : Void*, stack_bottom : Void*)
+    def self.set_stackbottom(thread : Thread, stack_bottom : Void*)
       sb = LibGC::StackBase.new
       sb.mem_base = stack_bottom
-      LibGC.set_stackbottom(thread_handle, pointerof(sb))
+      LibGC.set_stackbottom(thread.gc_thread_handler, pointerof(sb))
     end
   {% elsif LibGC.has_method?(:set_stackbottom) %}
     # this is necessary because Boehm GC does _not_ use `GC_stackbottom` on
@@ -373,9 +373,14 @@ module GC
       end
 
       {% if flag?(:preview_mt) %}
+        # update the thread stacks to their currently running fiber stacks, we
+        # only do it once _here_ instead of everytime we switch.
+        #
+        # A thread may not have a current fiber while starting, in which case we
+        # assume the GC knows about the thread and detected its stack.
         Thread.unsafe_each do |thread|
           if fiber = thread.current_fiber?
-            GC.set_stackbottom(thread.gc_thread_handler, fiber.@stack_bottom)
+            GC.set_stackbottom(thread, fiber.@stack_bottom)
           end
         end
       {% end %}