Skip to content

Commit 955015a

Browse files
Extract internal Process.block_signals helper (#16402)
Extract common behaviour between the two fork methods and encapsulates the signal blocking behaviour. As a result, the call site is much cleaner now.
1 parent bf90884 commit 955015a

File tree

2 files changed

+37
-49
lines changed

2 files changed

+37
-49
lines changed

src/crystal/system/unix/process.cr

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,25 @@ struct Crystal::System::Process
197197
def self.fork
198198
{% raise("Process fork is unsupported with multithreaded mode") if flag?(:preview_mt) %}
199199

200+
block_signals do
201+
case pid = lock_write { LibC.fork }
202+
when 0
203+
# forked process
204+
205+
::Process.after_fork_child_callbacks.each(&.call)
206+
207+
nil
208+
when -1
209+
# forking process: error
210+
raise RuntimeError.from_errno("fork")
211+
else
212+
# forking process: success
213+
pid
214+
end
215+
end
216+
end
217+
218+
private def self.block_signals(&)
200219
newmask = uninitialized LibC::SigsetT
201220
oldmask = uninitialized LibC::SigsetT
202221

@@ -211,26 +230,11 @@ struct Crystal::System::Process
211230
ret = LibC.pthread_sigmask(LibC::SIG_SETMASK, pointerof(newmask), pointerof(oldmask))
212231
raise RuntimeError.from_errno("Failed to disable signals") unless ret == 0
213232

214-
case pid = lock_write { LibC.fork }
215-
when 0
216-
# child:
217-
pid = nil
218-
219-
::Process.after_fork_child_callbacks.each(&.call)
220-
221-
# restore sigmask
222-
LibC.pthread_sigmask(LibC::SIG_SETMASK, pointerof(oldmask), nil)
223-
when -1
224-
# error:
225-
errno = Errno.value
226-
LibC.pthread_sigmask(LibC::SIG_SETMASK, pointerof(oldmask), nil)
227-
raise RuntimeError.from_os_error("fork", errno)
228-
else
229-
# parent:
233+
begin
234+
yield pointerof(oldmask)
235+
ensure
230236
LibC.pthread_sigmask(LibC::SIG_SETMASK, pointerof(oldmask), nil)
231237
end
232-
233-
pid
234238
end
235239

236240
# Duplicates the current process.

src/crystal/system/unix/spawn.cr

Lines changed: 15 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -59,41 +59,25 @@ struct Crystal::System::Process
5959
end
6060

6161
private def self.fork_for_exec
62-
newmask = uninitialized LibC::SigsetT
63-
oldmask = uninitialized LibC::SigsetT
64-
65-
# block signals while we fork, so the child process won't forward signals it
66-
# may receive to the parent through the signal pipe, but make sure to not
67-
# block stop-the-world signals as it appears to create deadlocks in glibc
68-
# for example; this is safe because these signal handlers musn't be
69-
# registered through `Signal.trap` but directly through `sigaction`.
70-
LibC.sigfillset(pointerof(newmask))
71-
LibC.sigdelset(pointerof(newmask), System::Thread.sig_suspend)
72-
LibC.sigdelset(pointerof(newmask), System::Thread.sig_resume)
73-
ret = LibC.pthread_sigmask(LibC::SIG_SETMASK, pointerof(newmask), pointerof(oldmask))
74-
raise RuntimeError.from_errno("Failed to disable signals") unless ret == 0
62+
block_signals do |sigmask|
63+
case pid = lock_write { LibC.fork }
64+
when 0
65+
# forked process
7566

76-
case pid = lock_write { LibC.fork }
77-
when 0
78-
# child:
79-
pid = nil
67+
Crystal::System::Signal.after_fork_before_exec
8068

81-
Crystal::System::Signal.after_fork_before_exec
69+
# reset sigmask (inherited on exec)
70+
LibC.sigemptyset(sigmask)
8271

83-
# reset sigmask (inherited on exec)
84-
LibC.sigemptyset(pointerof(newmask))
85-
LibC.pthread_sigmask(LibC::SIG_SETMASK, pointerof(newmask), nil)
86-
when -1
87-
# error:
88-
errno = Errno.value
89-
LibC.pthread_sigmask(LibC::SIG_SETMASK, pointerof(oldmask), nil)
90-
raise RuntimeError.from_os_error("fork", errno)
91-
else
92-
# parent:
93-
LibC.pthread_sigmask(LibC::SIG_SETMASK, pointerof(oldmask), nil)
72+
nil
73+
when -1
74+
# forking process: error
75+
raise RuntimeError.from_errno("fork")
76+
else
77+
# forking process: success
78+
pid
79+
end
9480
end
95-
96-
pid
9781
end
9882

9983
# This method is similar to `.replace` (used for `Process.exec`) with some

0 commit comments

Comments
 (0)