Skip to content

Commit

Permalink
Fix: add Thread.start/stop_world + remove GC.sig_suspend/resume
Browse files Browse the repository at this point in the history
Thread is now the entrypoint for stopping the world. Delegating to the
GC is an implementation detail.

The `sig_resume` and `sig_suspend` properties are now set on
Crystal::System::Thread and only for UNIX.
  • Loading branch information
ysbaddaden committed Jun 19, 2024
1 parent 75360de commit e105927
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 74 deletions.
8 changes: 8 additions & 0 deletions src/crystal/system/thread.cr
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,14 @@ class Thread
def resume : Nil
system_resume
end

def stop_world : Nil
GC.stop_world
end

def start_world : Nil
GC.start_world
end
end

require "./thread_linked_list"
Expand Down
30 changes: 25 additions & 5 deletions src/crystal/system/unix/pthread.cr
Original file line number Diff line number Diff line change
Expand Up @@ -172,13 +172,13 @@ module Crystal::System::Thread
# block all signals but sig_resume
mask = LibC::SigsetT.new
LibC.sigfillset(pointerof(mask))
LibC.sigdelset(pointerof(mask), GC.sig_resume)
LibC.sigdelset(pointerof(mask), sig_resume)

# suspend the thread until it receives the sig_resume signal
LibC.sigsuspend(pointerof(mask))
end
LibC.sigemptyset(pointerof(action.@sa_mask))
LibC.sigaction(GC.sig_suspend, pointerof(action), nil)
LibC.sigaction(sig_suspend, pointerof(action), nil)
end

private def self.install_sig_resume_signal_handler
Expand All @@ -188,13 +188,13 @@ module Crystal::System::Thread
# do nothing (a handler is still required to receive the signal)
end
LibC.sigemptyset(pointerof(action.@sa_mask))
LibC.sigaction(GC.sig_resume, pointerof(action), nil)
LibC.sigaction(sig_resume, pointerof(action), nil)
end

private def system_suspend : Nil
@suspended.set(false)

if LibC.pthread_kill(@system_handle, GC.sig_suspend) == -1
if LibC.pthread_kill(@system_handle, Thread.sig_suspend) == -1
System.panic("pthread_kill()")
end
end
Expand All @@ -206,10 +206,30 @@ module Crystal::System::Thread
end

private def system_resume : Nil
if LibC.pthread_kill(@system_handle, GC.sig_resume) == -1
if LibC.pthread_kill(@system_handle, Thread.sig_resume) == -1
System.panic("pthread_kill()")
end
end

protected class_property(sig_suspend : Int32) do
# follows bdwgc
{% if flag?(:linux) %}
LibC::SIGPWR
{% elsif LibC.has_constant?(:SIGRTMIN) %}
LibC::SIGRTMIN + 6
{% else %}
LibC::SIGXFSZ
{% end %}
end

protected class_property(sig_resume : Int32) do
# follows bdwgc
{% if LibC.has_constant?(:SIGRTMIN) %}
LibC::SIGRTMIN + 5
{% else %}
LibC::SIGXCPU
{% end %}
end
end

# In musl (alpine) the calls to unwind API segfaults
Expand Down
5 changes: 1 addition & 4 deletions src/crystal/system/win32/thread.cr
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,7 @@ module Crystal::System::Thread
private def system_wait_suspended : Nil
# context must be aligned on 16 bytes but we lack a mean to force the
# alignment on the struct, so we overallocate then realign the pointer:
#
# we only need to reserve UInt8[sizeof(LibC::CONTEXT) + 16] but that's
# invalid crystal
local = uninitialized LibC::CONTEXT[2]
local = uninitialized UInt8[sizeof(Tuple(LibC::CONTEXT, UInt8[15]))]
thread_context = Pointer(LibC::CONTEXT).new(local.to_unsafe.address &+ 15_u64 & ~15_u64)
thread_context.value.contextFlags = LibC::CONTEXT_FULL

Expand Down
25 changes: 0 additions & 25 deletions src/gc/boehm.cr
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,6 @@ lib LibGC
fun set_warn_proc = GC_set_warn_proc(WarnProc)
$warn_proc = GC_current_warn_proc : WarnProc

fun get_suspend_signal = GC_get_suspend_signal : Int
fun get_thr_restart_signal = GC_get_thr_restart_signal : Int
fun set_suspend_signal = GC_set_suspend_signal(Int) : Int
fun set_thr_restart_signal = GC_set_thr_restart_signal(Int) : Int

fun stop_world_external = GC_stop_world_external
fun start_world_external = GC_start_world_external
end
Expand Down Expand Up @@ -479,26 +474,6 @@ module GC
end
{% end %}

# :nodoc:
def sig_suspend : Int32
LibGC.get_suspend_signal
end

# :nodoc:
def sig_suspend=(value : Int32) : Int32
LibGC.set_suspend_signal(value)
end

# :nodoc:
def sig_resume : Int32
LibGC.get_thr_restart_signal
end

# :nodoc:
def sig_resume=(value : Int32) : Int32
LibGC.set_thr_restart_signal(value)
end

# :nodoc:
def self.stop_world : Nil
LibGC.stop_world_external
Expand Down
52 changes: 12 additions & 40 deletions src/gc/none.cr
Original file line number Diff line number Diff line change
Expand Up @@ -142,52 +142,24 @@ module GC
#
# This isn't a GC-safe stop-the-world implementation (it may allocate objects
# while stopping the world), but the guarantees are enough for the purpose of
# gc_none.
# gc_none. It could be GC-safe if Thread::LinkedList(T) became a struct, and
# Thread::Mutex either became a struct or provide low level abstraction
# methods that directly interact with syscalls (without allocating).
#
# Thread safety is guaranteed by the mutex in Crystal::PointerLinkedList:
# either a thread is starting and hasn't added itself to the list (it will
# block until it can acquire the lock), or is currently adding itself (the
# current thread will block until it can acquire the lock).
# Thread safety is guaranteed by the mutex in Thread::LinkedList: either a
# thread is starting and hasn't added itself to the list (it will block until
# it can acquire the lock), or is currently adding itself (the current thread
# will block until it can acquire the lock).
#
# In both cases there can't be a deadlock since we won't suspend another
# thread until it has successfuly added (or removed) itself to the linked
# list and released the lock, and the other thread won't progress until it
# can add (or remove) itself from the list.
# thread until it has successfuly added (or removed) itself to (from) the
# linked list and released the lock, and the other thread won't progress until
# it can add (or remove) itself from the list.
#
# Finally, we lock the mutex and keep it locked until we resume the world,
# so any thread waiting on the mutex will only be resumed when the world is
# Finally, we lock the mutex and keep it locked until we resume the world, so
# any thread waiting on the mutex will only be resumed when the world is
# resumed.

# :nodoc:
class_property(sig_suspend : Int32) do
# follows bdwgc
{% if flag?(:unix) %}
{% if flag?(:linux) %}
LibC::SIGPWR
{% elsif LibC.has_constant?(:SIGRTMIN) %}
LibC::SIGRTMIN + 6
{% else %}
LibC::SIGXFSZ
{% end %}
{% else %}
-1
{% end %}
end

# :nodoc:
class_property(sig_resume : Int32) do
# follows bdwgc
{% if flag?(:unix) %}
{% if LibC.has_constant?(:SIGRTMIN) %}
LibC::SIGRTMIN + 5
{% else %}
LibC::SIGXCPU
{% end %}
{% else %}
-1
{% end %}
end

# :nodoc:
def self.stop_world : Nil
current_thread = Thread.current
Expand Down

0 comments on commit e105927

Please sign in to comment.