Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
71 changes: 51 additions & 20 deletions src/crystal/once.cr
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
# initialized and is its responsibility to verify the initializer is executed
# only once and to fail on recursion.
#
# 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 %}
Expand All @@ -28,18 +31,28 @@
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.
@[AlwaysInline]
def self.once(flag : OnceState*, &block) : Nil
return if flag.value.initialized?
once(flag, block.pointer, block.closure_data)
Intrinsics.unreachable unless flag.value.initialized?
end

# :nodoc:
# Using @[NoInline] so LLVM optimizes for the hot path (var already
# initialized).
@[NoInline]
def self.once(flag : OnceState*, initializer : Void*) : Nil
def self.once(flag : OnceState*, initializer : Void*, closure_data : Void*) : Nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Why pass the function pointer and closure data as separate values instead of a Proc instance?

@@once_mutex.synchronize do
case flag.value
in .initialized?
return
in .uninitialized?
flag.value = :processing
Proc(Nil).new(initializer, Pointer(Void).null).call
Proc(Nil).new(initializer, closure_data).call
flag.value = :initialized
in .processing?
raise "Recursion while initializing class variables and/or constants"
Expand Down Expand Up @@ -71,7 +84,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);
Expand All @@ -82,41 +95,59 @@
# 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
@mutex = Mutex.new(:reentrant)
@rec = [] of Bool*
module Crystal
# :nodoc:
class OnceState
@mutex = Mutex.new(:reentrant)
@rec = [] of Bool*

@[NoInline]
def once(flag : Bool*, initializer : Void*)
return if flag.value
@[NoInline]
def once(flag : Bool*, initializer : Void*, closure_data : Void*)
return if flag.value

@mutex.synchronize do
if @rec.includes?(flag)
raise "Recursion while initializing class variables and/or constants"
end
@rec << flag
@mutex.synchronize do
return if flag.value

Proc(Nil).new(initializer, Pointer(Void).null).call
flag.value = true
if @rec.includes?(flag)
raise "Recursion while initializing class variables and/or constants"
end
@rec << flag

@rec.pop
Proc(Nil).new(initializer, closure_data).call
flag.value = true

@rec.pop
end
end
end

@@once_state = uninitialized OnceState

# :nodoc:
def self.once_state=(@@once_state : OnceState)
end

# :nodoc:
@[AlwaysInline]
def self.once(flag : Bool*, &block) : Nil
return if flag.value
@@once_state.once(flag, block.pointer, block.closure_data)
Intrinsics.unreachable unless flag.value
end
end

# :nodoc:
fun __crystal_once_init : Void*
Thread.init
Fiber.init
Crystal::OnceState.new.as(Void*)
(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 %}