Safe lazy initialization of class properties

scripts/generate_object_properties.cr

@@ -82,6 +82,33 @@ struct Generator
     TEXT
   end
 
+  # NOTE: we explicitly cast .as(typeof(yield)) to avoid type inference
+  # regressions, see https://github.com/crystal-lang/crystal/issues/15556
+  #
+  # FIXME: still parallel unsafe in case of parallel call to `setter` (unless
+  # type is a reference)  because we can't trust mixed union load/store to be
+  # safe operations.
+  def def_safe_getter(suffix = "")
+    <<-TEXT
+          {% if block %} #{@var_prefix}__{{var_name}}_lock = Crystal::Lock.new {% end %}
+
+          def #{@method_prefix}{{var_name}}#{suffix} {% if type %} : {{type}} {% end %}
+            {% if block %}
+              if (%value = #{@var_prefix}__{{var_name}}_lock.rlock { #{@var_prefix}{{var_name}} }).nil?
+                #{@var_prefix}__{{var_name}}_lock
+                  .lock { #{@var_prefix}{{var_name}} ||= {{yield}} }
+                  {% unless type %}.as(typeof({{yield}})){% end %}
+              else
+                %value
+              end
+            {% else %}
+              #{@var_prefix}{{var_name}}
+            {% end %}
+          end
+
+    TEXT
+  end
+
   def def_getter!
     <<-TEXT
           def #{@method_prefix}{{var_name}}? {% if type %} : {{type}}? {% end %}
@@ -116,7 +143,7 @@ struct Generator
       macro #{@macro_prefix}property(*names, &block)
         {% for name in names %}
     #{def_vars}
-    #{def_getter}
+    #{@macro_prefix == "class_" ? def_safe_getter : def_getter}
     #{def_setter}
         {% end %}
       end
@@ -128,7 +155,7 @@ struct Generator
       macro #{@macro_prefix}property?(*names, &block)
         {% for name in names %}
     #{def_vars}
-    #{def_getter "?"}
+    #{@macro_prefix == "class_" ? def_safe_getter("?") : def_getter("?")}
     #{def_setter}
         {% end %}
       end
@@ -159,6 +186,8 @@ File.open(output, "w") do |f|
   f.puts "#"
   f.puts "# DO NOT EDIT"
   f.puts
+  f.puts %(require "crystal/lock")
+  f.puts
   f.puts "class Object"
 
   g = Generator.new(f, "", "", "@", "#")
@@ -281,7 +310,7 @@ File.open(output, "w") do |f|
     macro class_getter(*names, &block)
       {% for name in names %}
   #{g.def_vars}
-  #{g.def_getter}
+  #{g.def_safe_getter}
       {% end %}
     end
 
@@ -309,7 +338,7 @@ File.open(output, "w") do |f|
     macro class_getter?(*names, &block)
       {% for name in names %}
   #{g.def_vars}
-  #{g.def_getter "?"}
+  #{g.def_safe_getter "?"}
       {% end %}
     end
 

spec/compiler/semantic/doc_spec.cr

@@ -678,8 +678,7 @@ describe "Semantic: doc" do
 
   it "expands record macro with comments (#16074)" do
     result = semantic <<-CRYSTAL, wants_doc: true
-      require "macros"
-      require "object/properties"
+      require "prelude"
 
       record Foo,
         # This is a multiline

src/crystal/lock.cr

@@ -0,0 +1,41 @@
+require "sync/mu"
+
+module Crystal
+  # :nodoc:
+  #
+  # Always checked alternative to Sync::Mutex and Sync::RWLock in a single and
+  # significantly smaller type (no crystal type id, no lock type, no reentrancy
+  # counter).
+  struct Lock
+    @mu = Sync::MU.new
+    @locked_by : Fiber?
+
+    def lock(&)
+      unless @mu.try_lock?
+        raise Sync::Error::Deadlock.new if deadlock?
+        @mu.lock_slow
+      end
+
+      begin
+        @locked_by = Fiber.current
+        yield
+      ensure
+        @locked_by = nil
+        @mu.unlock
+      end
+    end
+
+    def rlock(&)
+      @mu.rlock
+      begin
+        yield
+      ensure
+        @mu.runlock
+      end
+    end
+
+    private def deadlock?
+      @locked_by == Fiber.current
+    end
+  end
+end

src/crystal/pointer_linked_list.cr

@@ -7,8 +7,22 @@ struct Crystal::PointerLinkedList(T)
 
   module Node
     macro included
-      property previous : ::Pointer(self) = ::Pointer(self).null
-      property next : ::Pointer(self) = ::Pointer(self).null
+      @previous = Pointer(self).null
+      @next = Pointer(self).null
+
+      def previous : ::Pointer(self)
+        @previous
+      end
+
+      def previous=(@previous : ::Pointer(self))
+      end
+
+      def next : ::Pointer(self)
+        @next
+      end
+
+      def next=(@next : ::Pointer(self))
+      end
     end
   end
 

src/object/properties.cr

@@ -4,6 +4,8 @@
 #
 # DO NOT EDIT
 
+require "crystal/lock"
+
 class Object
   # Defines getter methods to access instance variables.
   #
@@ -334,10 +336,14 @@ class Object
         {% type = nil %}
       {% end %}
 
+      {% if block %} @@__{{var_name}}_lock = Crystal::Lock.new {% end %}
+
       def self.{{var_name}} {% if type %} : {{type}} {% end %}
         {% if block %}
-          if (%value = @@{{var_name}}).nil?
-            @@{{var_name}} = {{yield}}
+          if (%value = @@__{{var_name}}_lock.rlock { @@{{var_name}} }).nil?
+            @@__{{var_name}}_lock
+              .lock { @@{{var_name}} ||= {{yield}} }
+              {% unless type %}.as(typeof({{yield}})){% end %}
           else
             %value
           end
@@ -389,10 +395,14 @@ class Object
         {% type = nil %}
       {% end %}
 
+      {% if block %} @@__{{var_name}}_lock = Crystal::Lock.new {% end %}
+
       def self.{{var_name}}? {% if type %} : {{type}} {% end %}
         {% if block %}
-          if (%value = @@{{var_name}}).nil?
-            @@{{var_name}} = {{yield}}
+          if (%value = @@__{{var_name}}_lock.rlock { @@{{var_name}} }).nil?
+            @@__{{var_name}}_lock
+              .lock { @@{{var_name}} ||= {{yield}} }
+              {% unless type %}.as(typeof({{yield}})){% end %}
           else
             %value
           end
@@ -528,10 +538,14 @@ class Object
         {% type = nil %}
       {% end %}
 
+      {% if block %} @@__{{var_name}}_lock = Crystal::Lock.new {% end %}
+
       def self.{{var_name}} {% if type %} : {{type}} {% end %}
         {% if block %}
-          if (%value = @@{{var_name}}).nil?
-            @@{{var_name}} = {{yield}}
+          if (%value = @@__{{var_name}}_lock.rlock { @@{{var_name}} }).nil?
+            @@__{{var_name}}_lock
+              .lock { @@{{var_name}} ||= {{yield}} }
+              {% unless type %}.as(typeof({{yield}})){% end %}
           else
             %value
           end
@@ -569,10 +583,14 @@ class Object
         {% type = nil %}
       {% end %}
 
+      {% if block %} @@__{{var_name}}_lock = Crystal::Lock.new {% end %}
+
       def self.{{var_name}}? {% if type %} : {{type}} {% end %}
         {% if block %}
-          if (%value = @@{{var_name}}).nil?
-            @@{{var_name}} = {{yield}}
+          if (%value = @@__{{var_name}}_lock.rlock { @@{{var_name}} }).nil?
+            @@__{{var_name}}_lock
+              .lock { @@{{var_name}} ||= {{yield}} }
+              {% unless type %}.as(typeof({{yield}})){% end %}
           else
             %value
           end

src/sync/waiter.cr

@@ -10,7 +10,14 @@ module Sync
 
     include Crystal::PointerLinkedList::Node
 
-    property cv_mu : Pointer(MU)
+    @cv_mu : MU*
+
+    def cv_mu : MU*
+      @cv_mu
+    end
+
+    def cv_mu=(@cv_mu : MU*)
+    end
 
     def initialize(@type : Type, @cv_mu : Pointer(MU) = Pointer(MU).null)
       # protects against spurious wakeups (invalid manual fiber enqueues) that