Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions src/concurrent/scheduler.cr
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ class Scheduler
end
end

def self.create_fd_write_event(io : IO::FileDescriptor, edge_triggered : Bool = false)
def self.create_fd_write_event(handle : Crystal::System::FileHandle, edge_triggered : Bool = false)
flags = LibEvent2::EventFlags::Write
flags |= LibEvent2::EventFlags::Persist | LibEvent2::EventFlags::ET if edge_triggered
event = @@eb.new_event(io.fd, flags, io) do |s, flags, data|
fd_io = data.as(IO::FileDescriptor)
event = @@eb.new_event(handle.platform_specific, flags, handle) do |s, flags, data|
fd_handle = data.as(Crystal::System::FileHandle)
if flags.includes?(LibEvent2::EventFlags::Write)
fd_io.resume_write
fd_handle.resume_write
elsif flags.includes?(LibEvent2::EventFlags::Timeout)
fd_io.resume_write(timed_out: true)
fd_handle.resume_write(timed_out: true)
end
end
event
Expand All @@ -56,15 +56,15 @@ class Scheduler
event
end

def self.create_fd_read_event(io : IO::FileDescriptor, edge_triggered : Bool = false)
def self.create_fd_read_event(handle : Crystal::System::FileHandle, edge_triggered : Bool = false)
flags = LibEvent2::EventFlags::Read
flags |= LibEvent2::EventFlags::Persist | LibEvent2::EventFlags::ET if edge_triggered
event = @@eb.new_event(io.fd, flags, io) do |s, flags, data|
fd_io = data.as(IO::FileDescriptor)
event = @@eb.new_event(handle.platform_specific, flags, handle) do |s, flags, data|
fd_handle = data.as(Crystal::System::FileHandle)
if flags.includes?(LibEvent2::EventFlags::Read)
fd_io.resume_read
fd_handle.resume_read
elsif flags.includes?(LibEvent2::EventFlags::Timeout)
fd_io.resume_read(timed_out: true)
fd_handle.resume_read(timed_out: true)
end
end
event
Expand Down
81 changes: 81 additions & 0 deletions src/crystal/system/file_handle.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
class Crystal::System::FileHandle
include IO

# Create a new `FileHandle` using the platform-specific representation of this
# handle.
#
# On unix-like platforms this constructor takes an Int32 file descriptor.
# def self.new(platform_specific) : FileHandle

# Returns the platform-specific representation of this handle.
#
# On unix-like platforms this returns an Int32 file descriptor.
# def platform_specific
Copy link
Collaborator

Choose a reason for hiding this comment

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

platform_specific is the worst possible naming. If it's supposed to represent the underlying system data, then maybe raw or even handle if we go with windows naming (bad idea: Windows is one target, POSIX is all others) or just a plain #to_unsafe so we can pass @handle directly to C functions instead of the horrifying @handle.platform_specific we see everywhere.

In fact, I don't even see any reason to abstract the naming here: any POSIX system specifics will expect a file descriptor, and Windows system specifics will expect a HANDLE.

Trying to abstrtact this is actually confusing and misleading. We could have the false impression that LibC.tcsetattr(@handle.platform_specific, ...) will just work on Windows for example.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you're right that it's bad naming, but I don't think that #to_unsafe is a good way to handle this. It's way too magic to pass a crystal object and it magically gets converted into a file descriptor. I think automatically calling to_unsafe is way too magic in general...

I think you're probably right about naming it fd on posix and handle on windows.


# Reads at most `slice.size` bytes from this `FileHandle` into *slice*.
# Returns the number of bytes read.
# def read(slice : Bytes) : Int32

# Writes the contents of *slice* into this `FileHandle`.
# def write(slice : Bytes) : Nil

# Used by the scheduler to call back to the `FileHandle` once a read is ready.
# def resume_read(timed_out : Bool = false) : Nil

# Used by the scheduler to call back to the `FileHandle` once a write is
# ready.
# def resume_write(timed_out : Bool = false) : Nil

# Returns true if this `FileHandle` has been closed.
# def closed? : Bool

# Closes the `FileHandle`.
# def close : Nil

# Returns true if the `FileHandle` uses blocking IO.
# def blocking? : Bool

# Sets whether this `FileHandle` uses blocking IO.
# def blocking=(value : Bool) : Bool

# Returns true if this `FileHandle` is closed when `Process.exec` is called.
# def close_on_exec? : Bool

# Sets if this `FileHandle` is closed when `Process.exec` is called.
# def close_on_exec=(value : Bool) : Bool

# Returns the time to wait when reading before raising an `IO::Timeout`.
# def read_timeout : Time::Span?

# Sets the time to wait when reading before raising an `IO::Timeout`.
# def read_timeout=(timeout : Time::Span?) : Time::Span?

# Returns the time to wait when writing before raising an `IO::Timeout`.
# def write_timeout : Time::Span?

# Sets the time to wait when writing before raising an `IO::Timeout`.
# def write_timeout=(timeout : Time::Span?) : Time::Span?

# Seeks to a given offset relative to either the beginning, current position,
# or end - depending on *whence*. Returns the new position in the file
# measured in bytes from the beginning of the file.
# def seek(offset : Number, whence : IO::Seek = IO::Seek::Set) : Int64

# Returns true if this `FileHandle` is a handle of a terminal device (tty).
# TODO: rename this to `terminal?`
# def tty? : Bool

# Modifies this `FileHandle` to be a handle of the same resource as *other*.
# def reopen(other : FileHandle) : FileHandle

# Returns a `File::Stat` object containing information about the file that
# this `FileHandle` represents.
# def stat : File::Stat

# Implement `IO#rewind` using `seek` for all implementations.
def rewind
seek(0, IO::Seek::Set)
end
end

require "./unix/file_handle"
172 changes: 172 additions & 0 deletions src/crystal/system/unix/file_handle.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
require "c/fcntl"
require "io/syscall"

class Crystal::System::FileHandle
include IO::Syscall

@fd : Int32

@read_event : Event::Event?
@write_event : Event::Event?

@closed = false

def initialize(platform_specific : Int32)
@fd = platform_specific
end

def platform_specific : Int32
@fd
end

def read(slice : Bytes) : Int32
read_syscall_helper(slice, "Error reading file") do
# `to_i32` is acceptable because `Slice#size` is a Int32
LibC.read(@fd, slice, slice.size).to_i32
end
end

def write(slice : Bytes) : Nil
write_syscall_helper(slice, "Error writing file") do |slice|
LibC.write(@fd, slice, slice.size).tap do |return_code|
if return_code == -1 && Errno.value == Errno::EBADF
raise IO::Error.new "File not open for writing"
end
end
end
end

def closed? : Bool
@closed
end

def close : Nil
return if @closed

err = nil
if LibC.close(@fd) != 0
case Errno.value
when Errno::EINTR, Errno::EINPROGRESS
# ignore
else
err = Errno.new("Error closing file")
end
end

@closed = true

@read_event.try &.free
@read_event = nil

@write_event.try &.free
@write_event = nil

reschedule_waiting

raise err if err
end

def blocking? : Bool
(fcntl(LibC::F_GETFL) & LibC::O_NONBLOCK) == 0
end

def blocking=(value : Bool) : Bool
flags = fcntl(LibC::F_GETFL)
if value
flags &= ~LibC::O_NONBLOCK
else
flags |= LibC::O_NONBLOCK
end
fcntl(LibC::F_SETFL, flags)

value
end

def close_on_exec? : Bool
(fcntl(LibC::F_GETFD) & LibC::FD_CLOEXEC) == LibC::FD_CLOEXEC
end

def close_on_exec=(value : Bool) : Bool
flags = fcntl(LibC::F_GETFD)
if value
flags |= LibC::FD_CLOEXEC
else
flags &= ~LibC::FD_CLOEXEC
end
fcntl(LibC::F_SETFD, flags)

value
end

def seek(offset : Number, whence : IO::Seek = IO::Seek::Set) : Int64
check_open

seek_value = LibC.lseek(@fd, offset.to_i64, whence)

if seek_value == -1
raise Errno.new "Unable to seek"
end

seek_value.to_i64
end

def tty? : Bool
LibC.isatty(@fd) == 1
end

def reopen(other : FileHandle) : FileHandle
{% if LibC.methods.includes? "dup3".id %}
# dup doesn't copy the CLOEXEC flag, so copy it manually using dup3
flags = other.close_on_exec? ? LibC::O_CLOEXEC : 0
if LibC.dup3(other.platform_specific, self.platform_specific, flags) == -1
raise Errno.new("Could not reopen file descriptor")
end
{% else %}
# dup doesn't copy the CLOEXEC flag, copy it manually to the new
Copy link
Collaborator

Choose a reason for hiding this comment

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

Misplaced comment.

Copy link
Member Author

@RX14 RX14 Oct 1, 2017

Choose a reason for hiding this comment

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

I'm confused, it's missing a word but why is it misplaced? It's documenting the implementation so it should be inside the method.

if LibC.dup2(other.platform_specific, self.platform_specific) == -1
raise Errno.new("Could not reopen file descriptor")
end

if other.close_on_exec?
self.close_on_exec = true
end
{% end %}

# We are now pointing to a new file descriptor, we need to re-register
# events with libevent and enqueue readers and writers again.
@read_event.try &.free
@read_event = nil

@write_event.try &.free
@write_event = nil

reschedule_waiting

other
end

def stat : File::Stat
if LibC.fstat(@fd, out stat) != 0
raise Errno.new("Unable to get stat")
end
File::Stat.new(stat)
end

private def fcntl(cmd, arg = 0)
LibC.fcntl(@fd, cmd, arg).tap do |ret|
raise Errno.new("fcntl() failed") if ret == -1
end
end

private def add_read_event(timeout = @read_timeout)
event = @read_event ||= Scheduler.create_fd_read_event(self)
event.add(timeout)
nil
end

private def add_write_event(timeout = @write_timeout)
event = @write_event ||= Scheduler.create_fd_write_event(self)
event.add(timeout)
nil
end
end
6 changes: 3 additions & 3 deletions src/file.cr
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class File < IO::FileDescriptor

@path = filename
self.set_encoding(encoding, invalid: invalid) if encoding
super(fd, blocking: true)
super(Crystal::System::FileHandle.new(fd), blocking: true)
end

protected def open_flag(mode)
Expand Down Expand Up @@ -620,7 +620,7 @@ class File < IO::FileDescriptor
# for writing.
def truncate(size = 0)
flush
code = LibC.ftruncate(fd, size)
code = LibC.ftruncate(@handle.platform_specific, size)
if code != 0
raise Errno.new("Error truncating file '#{path}'")
end
Expand All @@ -644,7 +644,7 @@ class File < IO::FileDescriptor
raise ArgumentError.new("Bytesize out of bounds")
end

io = PReader.new(fd, offset, bytesize)
io = PReader.new(@handle.platform_specific, offset, bytesize)
yield io ensure io.close
end

Expand Down
2 changes: 1 addition & 1 deletion src/file/flock.cr
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class File
private def flock(op : LibC::FlockOp, blocking : Bool = true)
op |= LibC::FlockOp::NB unless blocking

if LibC.flock(@fd, op) != 0
if LibC.flock(@handle.platform_specific, op) != 0
raise Errno.new("flock")
end

Expand Down
16 changes: 8 additions & 8 deletions src/io/console.cr
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ class IO::FileDescriptor
# This will prevent displaying back to the user what they enter on the terminal.
# Only call this when this IO is a TTY, such as a not redirected stdin.
def noecho!
if LibC.tcgetattr(fd, out mode) != 0
if LibC.tcgetattr(@handle.platform_specific, out mode) != 0
Copy link
Member Author

Choose a reason for hiding this comment

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

A lot of this stuff still needs to be platform abstracted.

raise Errno.new "can't set IO#noecho!"
end
noecho_from_tc_mode!
end

macro noecho_from_tc_mode!
mode.c_lflag &= ~(Termios::LocalMode.flags(ECHO, ECHOE, ECHOK, ECHONL).value)
LibC.tcsetattr(fd, Termios::LineControl::TCSANOW, pointerof(mode))
LibC.tcsetattr(@handle.platform_specific, Termios::LineControl::TCSANOW, pointerof(mode))
end

# Enable character processing for the duration of the given block.
Expand All @@ -50,7 +50,7 @@ class IO::FileDescriptor
# the program on a newline.
# Only call this when this IO is a TTY, such as a not redirected stdin.
def cooked!
if LibC.tcgetattr(fd, out mode) != 0
if LibC.tcgetattr(@handle.platform_specific, out mode) != 0
raise Errno.new "can't set IO#cooked!"
end
cooked_from_tc_mode!
Expand All @@ -69,7 +69,7 @@ class IO::FileDescriptor
Termios::LocalMode::ICANON |
Termios::LocalMode::ISIG |
Termios::LocalMode::IEXTEN).value
LibC.tcsetattr(fd, Termios::LineControl::TCSANOW, pointerof(mode))
LibC.tcsetattr(@handle.platform_specific, Termios::LineControl::TCSANOW, pointerof(mode))
end

# Enable raw mode for the duration of the given block.
Expand All @@ -88,7 +88,7 @@ class IO::FileDescriptor
# is done by the terminal.
# Only call this when this IO is a TTY, such as a not redirected stdin.
def raw!
if LibC.tcgetattr(fd, out mode) != 0
if LibC.tcgetattr(@handle.platform_specific, out mode) != 0
raise Errno.new "can't set IO#raw!"
end

Expand All @@ -97,18 +97,18 @@ class IO::FileDescriptor

macro raw_from_tc_mode!
LibC.cfmakeraw(pointerof(mode))
LibC.tcsetattr(fd, Termios::LineControl::TCSANOW, pointerof(mode))
LibC.tcsetattr(@handle.platform_specific, Termios::LineControl::TCSANOW, pointerof(mode))
end

private def preserving_tc_mode(msg)
if LibC.tcgetattr(fd, out mode) != 0
if LibC.tcgetattr(@handle.platform_specific, out mode) != 0
raise Errno.new msg
end
before = mode
begin
yield mode
ensure
LibC.tcsetattr(fd, Termios::LineControl::TCSANOW, pointerof(before))
LibC.tcsetattr(@handle.platform_specific, Termios::LineControl::TCSANOW, pointerof(before))
end
end
end
Loading