Skip to content

Commit

Permalink
Don't involve evloop after fork in System::Process.spawn (UNIX) (#14974)
Browse files Browse the repository at this point in the history
Refactors spawning child processes on UNIX that relies on fork/exec to not involve the event loop after fork and before exec. We still continue to rely on the eventloop in the parent process, of course.

* Extract Crystal::System::FileDescriptor.system_pipe (UNIX)
* Fix: avoid evloop after fork to report failures in Process.spawn (UNIX)
* Fix: don't involve evloop in System::Process.reopen_io (UNIX)

This is called after fork before exec to reopen the stdio. We can
leverage some abstractions (set blocking, unset cloexec) but musn't call
to methods that involve the evloop and would mess with the parent
evloop.
  • Loading branch information
ysbaddaden authored Sep 6, 2024
1 parent ef306fb commit 136f85e
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 29 deletions.
28 changes: 23 additions & 5 deletions src/crystal/system/unix/file_descriptor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,14 @@ module Crystal::System::FileDescriptor
end

def self.pipe(read_blocking, write_blocking)
pipe_fds = system_pipe
r = IO::FileDescriptor.new(pipe_fds[0], read_blocking)
w = IO::FileDescriptor.new(pipe_fds[1], write_blocking)
w.sync = true
{r, w}
end

def self.system_pipe : StaticArray(LibC::Int, 2)
pipe_fds = uninitialized StaticArray(LibC::Int, 2)

{% if LibC.has_method?(:pipe2) %}
Expand All @@ -219,11 +227,7 @@ module Crystal::System::FileDescriptor
end
{% end %}

r = IO::FileDescriptor.new(pipe_fds[0], read_blocking)
w = IO::FileDescriptor.new(pipe_fds[1], write_blocking)
w.sync = true

{r, w}
pipe_fds
end

def self.pread(file, buffer, offset)
Expand Down Expand Up @@ -255,6 +259,20 @@ module Crystal::System::FileDescriptor
io
end

# Helper to write *size* values at *pointer* to a given *fd*.
def self.write_fully(fd : LibC::Int, pointer : Pointer, size : Int32 = 1) : Nil
write_fully(fd, Slice.new(pointer, size).unsafe_slice_of(UInt8))
end

# Helper to fully write a slice to a given *fd*.
def self.write_fully(fd : LibC::Int, slice : Slice(UInt8)) : Nil
until slice.size == 0
size = LibC.write(fd, slice, slice.size)
break if size == -1
slice += size
end
end

private def system_echo(enable : Bool, mode = nil)
new_mode = mode || FileDescriptor.tcgetattr(fd)
flags = LibC::ECHO | LibC::ECHOE | LibC::ECHOK | LibC::ECHONL
Expand Down
53 changes: 29 additions & 24 deletions src/crystal/system/unix/process.cr
Original file line number Diff line number Diff line change
Expand Up @@ -231,44 +231,47 @@ struct Crystal::System::Process
end

def self.spawn(command_args, env, clear_env, input, output, error, chdir)
reader_pipe, writer_pipe = IO.pipe
r, w = FileDescriptor.system_pipe

pid = self.fork(will_exec: true)
if !pid
LibC.close(r)
begin
reader_pipe.close
writer_pipe.close_on_exec = true
self.try_replace(command_args, env, clear_env, input, output, error, chdir)
writer_pipe.write_byte(1)
writer_pipe.write_bytes(Errno.value.to_i)
byte = 1_u8
errno = Errno.value.to_i32
FileDescriptor.write_fully(w, pointerof(byte))
FileDescriptor.write_fully(w, pointerof(errno))
rescue ex
writer_pipe.write_byte(0)
byte = 0_u8
message = ex.inspect_with_backtrace
writer_pipe.write_bytes(message.bytesize)
writer_pipe << message
writer_pipe.close
FileDescriptor.write_fully(w, pointerof(byte))
FileDescriptor.write_fully(w, message.to_slice)
ensure
LibC.close(w)
LibC._exit 127
end
end

writer_pipe.close
LibC.close(w)
reader_pipe = IO::FileDescriptor.new(r, blocking: false)

begin
case reader_pipe.read_byte
when nil
# Pipe was closed, no error
when 0
# Error message coming
message_size = reader_pipe.read_bytes(Int32)
if message_size > 0
message = String.build(message_size) { |io| IO.copy(reader_pipe, io, message_size) }
end
reader_pipe.close
message = reader_pipe.gets_to_end
raise RuntimeError.new("Error executing process: '#{command_args[0]}': #{message}")
when 1
# Errno coming
errno = Errno.new(reader_pipe.read_bytes(Int32))
self.raise_exception_from_errno(command_args[0], errno)
# can't use IO#read_bytes(Int32) because we skipped system/network
# endianness check when writing the integer while read_bytes would;
# we thus read it in the same as order as written
buf = uninitialized StaticArray(UInt8, 4)
reader_pipe.read_fully(buf.to_slice)
raise_exception_from_errno(command_args[0], Errno.new(buf.unsafe_as(Int32)))
else
raise RuntimeError.new("BUG: Invalid error response received from subprocess")
end
Expand Down Expand Up @@ -339,15 +342,17 @@ struct Crystal::System::Process

private def self.reopen_io(src_io : IO::FileDescriptor, dst_io : IO::FileDescriptor)
if src_io.closed?
dst_io.close
return
end
dst_io.file_descriptor_close
else
src_io = to_real_fd(src_io)

src_io = to_real_fd(src_io)
# dst_io.reopen(src_io)
ret = LibC.dup2(src_io.fd, dst_io.fd)
raise IO::Error.from_errno("dup2") if ret == -1

dst_io.reopen(src_io)
dst_io.blocking = true
dst_io.close_on_exec = false
dst_io.blocking = true
dst_io.close_on_exec = false
end
end

private def self.to_real_fd(fd : IO::FileDescriptor)
Expand Down

0 comments on commit 136f85e

Please sign in to comment.