Add fiber safety to __crystal_once & class_[getter|property]?(&) macros

spec/std/socket/spec_helper.cr

@@ -2,7 +2,11 @@ require "spec"
 require "socket"
 
 module SocketSpecHelper
-  class_getter?(supports_ipv6 : Bool) do
+  @@supports_ipv6 : Bool?
+
+  class_getter?(supports_ipv6 : Bool) { detect_supports_ipv6? }
+
+  private def self.detect_supports_ipv6? : Bool
     TCPServer.open("::1", 0) { return true }
     false
   rescue Socket::Error

spec/support/time.cr

@@ -72,7 +72,9 @@ end
     # Enable the `SeTimeZonePrivilege` privilege before changing the system time
     # zone. This is necessary because the privilege is by default granted but
     # disabled for any new process. This only needs to be done once per run.
-    class_getter? time_zone_privilege_enabled : Bool do
+    class_getter?(time_zone_privilege_enabled : Bool) { detect_time_zone_privilege_enabled? }
+
+    private def self.detect_time_zone_privilege_enabled? : Bool
       if LibC.LookupPrivilegeValueW(nil, SeTimeZonePrivilege, out time_zone_luid) == 0
         raise RuntimeError.from_winerror("LookupPrivilegeValueW")
       end

src/crystal/once.cr

@@ -7,12 +7,11 @@
 # - `__crystal_once`: called each time a constant or class variable has to be
 #   initialized and is its responsibility to verify the initializer is executed
 #   only once and to fail on recursion.
-
-# In multithread mode a mutex is used to avoid race conditions between threads.
 #
-# On Win32, `Crystal::System::FileDescriptor#@@reader_thread` spawns a new
-# thread even without the `preview_mt` flag, and the thread can also reference
-# Crystal constants, leading to race conditions, so we always enable the mutex.
+# Also defines the `Crystal.once(flag, &)` method used to protect lazy
+# initialization of class getters & properties.
+#
+# A `Mutex` is used to avoid race conditions between threads and fibers.
 
 {% if compare_versions(Crystal::VERSION, "1.16.0-dev") >= 0 %}
   # This implementation uses an enum over the initialization flag pointer for
@@ -22,28 +21,32 @@
     # :nodoc:
     enum OnceState : Int8
       Processing    = -1
-      Uninitialized = 0
-      Initialized   = 1
+      Uninitialized =  0
+      Initialized   =  1
     end
 
-    {% if flag?(:preview_mt) || flag?(:win32) %}
-      @@once_mutex = uninitialized Mutex
+    @@once_mutex = uninitialized Mutex
 
-      # :nodoc:
-      def self.once_mutex=(@@once_mutex : Mutex)
-      end
-    {% end %}
+    # :nodoc:
+    def self.once_mutex=(@@once_mutex : Mutex)
+    end
 
     # :nodoc:
+    #
+    # Identical to `__crystal_once` but takes a block with possibly closured
+    # data. Used by `class_[getter|property](declaration, &block)` for example.
+    def self.once(flag : OnceState*, &) : Nil
+      return if flag.value.initialized?
+      once_exec(flag) { yield }
+    end
+
+    # :nodoc:
+    #
     # Using @[NoInline] so LLVM optimizes for the hot path (var already
     # initialized).
     @[NoInline]
-    def self.once(flag : OnceState*, initializer : Void*) : Nil
-      {% if flag?(:preview_mt) || flag?(:win32) %}
-        @@once_mutex.synchronize { once_exec(flag, initializer) }
-      {% else %}
-        once_exec(flag, initializer)
-      {% end %}
+    def self.once(flag : OnceState*, initializer : Void*, closure_data : Void*) : Nil
+      once_exec(flag) { Proc(Nil).new(initializer, closure_data).call }
 
       # safety check, and allows to safely call `Intrinsics.unreachable` in
       # `__crystal_once`
@@ -53,25 +56,27 @@
       end
     end
 
-    private def self.once_exec(flag : OnceState*, initializer : Void*) : Nil
-      case flag.value
-      in .initialized?
-        return
-      in .uninitialized?
-        flag.value = :processing
-        Proc(Nil).new(initializer, Pointer(Void).null).call
-        flag.value = :initialized
-      in .processing?
-        raise "Recursion while initializing class variables and/or constants"
+    private def self.once_exec(flag, &)
+      @@once_mutex.synchronize do
+        case flag.value
+        in .initialized?
+          return
+        in .uninitialized?
+          flag.value = OnceState::Processing
+          yield
+          flag.value = OnceState::Initialized
+        in .processing?
+          raise "Recursion while initializing class variables and/or constants"
+        end
       end
     end
   end
 
   # :nodoc:
   fun __crystal_once_init : Nil
-    {% if flag?(:preview_mt) || flag?(:win32) %}
-      Crystal.once_mutex = Mutex.new(:reentrant)
-    {% end %}
+    Thread.init
+    Fiber.init
+    Crystal.once_mutex = Mutex.new(:reentrant)
   end
 
   # :nodoc:
@@ -83,7 +88,7 @@
   fun __crystal_once(flag : Crystal::OnceState*, initializer : Void*) : Nil
     return if flag.value.initialized?
 
-    Crystal.once(flag, initializer)
+    Crystal.once(flag, initializer, Pointer(Void).null)
 
     # tell LLVM that it can optimize away repeated `__crystal_once` calls for
     # this global (e.g. repeated access to constant in a single funtion);
@@ -94,49 +99,72 @@
   # This implementation uses a global array to store the initialization flag
   # pointers for each value to find infinite loops and raise an error.
 
-  # :nodoc:
-  class Crystal::OnceState
-    @rec = [] of Bool*
-
-    @[NoInline]
-    def once(flag : Bool*, initializer : Void*)
-      unless flag.value
-        if @rec.includes?(flag)
-          raise "Recursion while initializing class variables and/or constants"
-        end
-        @rec << flag
+  module Crystal
+    # :nodoc:
+    class OnceState
+      @mutex = Mutex.new(:reentrant)
+      @rec = [] of Bool*
 
-        Proc(Nil).new(initializer, Pointer(Void).null).call
-        flag.value = true
+      def once(flag : Bool*, &)
+        return if flag.value
+        once_exec(flag) { yield }
+      end
 
-        @rec.pop
+      @[NoInline]
+      def once(flag : Bool*, initializer : Void*, closure_data : Void*)
+        once_exec(flag) { Proc(Nil).new(initializer, closure_data).call }
       end
-    end
 
-    {% if flag?(:preview_mt) || flag?(:win32) %}
-      @mutex = Mutex.new(:reentrant)
+      private def once_exec(flag, &)
+        @mutex.synchronize do
+          return if flag.value
 
-      @[NoInline]
-      def once(flag : Bool*, initializer : Void*)
-        unless flag.value
-          @mutex.synchronize do
-            previous_def
+          if @rec.includes?(flag)
+            raise "Recursion while initializing class variables and/or constants"
           end
+          @rec << flag
+
+          yield
+          flag.value = true
+
+          @rec.pop
         end
       end
-    {% end %}
+    end
+
+    @@once_state = uninitialized OnceState
+
+    # :nodoc:
+    def self.once_state=(@@once_state : OnceState)
+    end
+
+    # :nodoc:
+    def self.once(flag : Bool*, &) : Nil
+      return if flag.value
+      @@once_state.once(flag) { yield }
+    end
   end
 
   # :nodoc:
   fun __crystal_once_init : Void*
-    Crystal::OnceState.new.as(Void*)
+    Thread.init
+    Fiber.init
+    (Crystal.once_state = Crystal::OnceState.new).as(Void*)
   end
 
   # :nodoc:
   @[AlwaysInline]
   fun __crystal_once(state : Void*, flag : Bool*, initializer : Void*)
     return if flag.value
-    state.as(Crystal::OnceState).once(flag, initializer)
+    state.as(Crystal::OnceState).once(flag, initializer, Pointer(Void).null)
     Intrinsics.unreachable unless flag.value
   end
 {% end %}
+
+{% if flag?(:interpreted) %}
+  # make sure to initialize the mutex so we can use Crystal.once in the
+  # class_[getter|property]? macros; the compiler does the call by itself, but
+  # the interpreter doesn't (it doesn't use __crystal_once to protect the
+  # initialization of constants and class vars).
+  __crystal_once_init
+{% end %}

src/crystal/system/thread.cr

@@ -2,6 +2,8 @@
 module Crystal::System::Thread
   # alias Handle
 
+  # def self.init : Nil
+
   # def self.new_handle(thread_obj : ::Thread) : Handle
 
   # def self.current_handle : Handle
@@ -48,7 +50,16 @@ class Thread
   include Crystal::System::Thread
 
   # all thread objects, so the GC can see them (it doesn't scan thread locals)
-  protected class_getter(threads) { Thread::LinkedList(Thread).new }
+  @@threads = uninitialized Thread::LinkedList(Thread)
+
+  protected def self.threads : Thread::LinkedList(Thread)
+    @@threads
+  end
+
+  def self.init : Nil
+    @@threads = Thread::LinkedList(Thread).new
+    Crystal::System::Thread.init
+  end
 
   @system_handle : Crystal::System::Thread::Handle
   @exception : Exception?

src/crystal/system/unix/pthread.cr

@@ -26,6 +26,16 @@ module Crystal::System::Thread
     raise RuntimeError.from_os_error("pthread_create", Errno.new(ret)) unless ret == 0
   end
 
+  def self.init : Nil
+    {% if flag?(:musl) %}
+      @@main_handle = current_handle
+    {% elsif flag?(:openbsd) || flag?(:android) %}
+      ret = LibC.pthread_key_create(out current_key, nil)
+      raise RuntimeError.from_os_error("pthread_key_create", Errno.new(ret)) unless ret == 0
+      @@current_key = current_key
+    {% end %}
+  end
+
   def self.thread_proc(data : Void*) : Void*
     th = data.as(::Thread)
 
@@ -53,13 +63,7 @@ module Crystal::System::Thread
   # Android appears to support TLS to some degree, but executables fail with
   # an underaligned TLS segment, see https://github.com/crystal-lang/crystal/issues/13951
   {% if flag?(:openbsd) || flag?(:android) %}
-    @@current_key : LibC::PthreadKeyT
-
-    @@current_key = begin
-      ret = LibC.pthread_key_create(out current_key, nil)
-      raise RuntimeError.from_os_error("pthread_key_create", Errno.new(ret)) unless ret == 0
-      current_key
-    end
+    @@current_key = uninitialized LibC::PthreadKeyT
 
     def self.current_thread : ::Thread
       if ptr = LibC.pthread_getspecific(@@current_key)
@@ -84,11 +88,18 @@ module Crystal::System::Thread
     end
   {% else %}
     @[ThreadLocal]
-    class_property current_thread : ::Thread { ::Thread.new }
+    @@current_thread : ::Thread?
+
+    def self.current_thread : ::Thread
+      @@current_thread ||= ::Thread.new
+    end
 
     def self.current_thread? : ::Thread?
       @@current_thread
     end
+
+    def self.current_thread=(@@current_thread : ::Thread)
+    end
   {% end %}
 
   def self.sleep(time : ::Time::Span) : Nil
@@ -169,7 +180,7 @@ module Crystal::System::Thread
   end
 
   {% if flag?(:musl) %}
-    @@main_handle : Handle = current_handle
+    @@main_handle = uninitialized Handle
 
     def self.current_is_main?
       current_handle == @@main_handle

src/crystal/system/wasi/thread.cr

@@ -1,6 +1,9 @@
 module Crystal::System::Thread
   alias Handle = Nil
 
+  def self.init : Nil
+  end
+
   def self.new_handle(thread_obj : ::Thread) : Handle
     raise NotImplementedError.new("Crystal::System::Thread.new_handle")
   end
@@ -13,7 +16,16 @@ module Crystal::System::Thread
     raise NotImplementedError.new("Crystal::System::Thread.yield_current")
   end
 
-  class_property current_thread : ::Thread { ::Thread.new }
+  def self.current_thread : ::Thread
+    @@current_thread ||= ::Thread.new
+  end
+
+  def self.current_thread? : ::Thread?
+    @@current_thread
+  end
+
+  def self.current_thread=(@@current_thread : ::Thread)
+  end
 
   def self.sleep(time : ::Time::Span) : Nil
     req = uninitialized LibC::Timespec