Skip to content

Commit

Permalink
Codedb ffi io (#3619)
Browse files Browse the repository at this point in the history
* add basic support for additional IO functions

* add back dup2 function definition

* add convenience methods for errno handling

* add O_CLOEXEC to list for File IO

* add a few more functions like ftruncate to the list

* hack away and rejigger things to get IO booting using mostly FFI calls

* make some fixes to pass more specs

* passes most read specs

* add spec for negative IO#pos when 'unget'ing at start of stream

* more fixes to pass ungetc/ungetbyte/pos/read specs; all pass

* fix #getc to handle encodings properly; passes chars_spec

* fix read_to_separator to handle skipping properly

* specs for requesting a negative-sized pointer which causes SEGV

* return NULL for a MemoryPointer.new with a negative size

* add basic support for additional IO functions

* add back dup2 function definition

* add convenience methods for errno handling

* add O_CLOEXEC to list for File IO

* add a few more functions like ftruncate to the list

* hack away and rejigger things to get IO booting using mostly FFI calls

* make some fixes to pass more specs

* passes most read specs

* add spec for negative IO#pos when 'unget'ing at start of stream

* more fixes to pass ungetc/ungetbyte/pos/read specs; all pass

* fix #getc to handle encodings properly; passes chars_spec

* fix read_to_separator to handle skipping properly

* specs for requesting a negative-sized pointer which causes SEGV

* return NULL for a MemoryPointer.new with a negative size

* raise IOError on malloc failure; force returned strings to binary

* force strings to binary for handling #gets with separators and limits; remove debug prints

* remove debug prints again - ps i hate git

* cleanup var use to make specs happy

* fix initialize specs

* remove some commented out and dead code

* switch from C++-based #seek to FFI seek

* redefine STDIN/STDOUT/STDERR to use new IO obj

* switch to using FFI-bound #write command

* include sync as part of fields to move to new IO obj

* mark rubinius as non-compliant for buffering writes; impl detail anyway

* to swap in new IO we need it to happen early enough in the load order before STDIN/etc are used

* need to redefine STDIN/etc and /etc when we swap in new IO object

* begin refactoring IO internals to make support and expansion simpler hopefully

* simplify by taking advantage of existing class method

* add a convenience method for testing call failure

* add #pipe function

* continue refactoring to support pipes and reopening FDs

* baby steps toward making #dup work

* use #descriptor accessor instead of using ivar directly

* make #ensure_ methods public; buffered read should return nil when EOF

* fix error message to use proper local var

* fix #new_pipe so handle encodings correctly

* set encodings for 'left hand side' pipes

* fix #dup specs and add a debugging method

* fix #eof specs

* fix #getbyte spec

* fix several IO#reopen_path bugs particularly improper Errno usage

* use a FFI convenience method for checking call failure

* add spec to test EOF behavior with IO#reopen

* fix IO#reopen to handle EOF correctly

* fix some eof specs with just one to go

* fix setting EOF when seeking

* make any seek on a Pipe raise Errno::ESPIPE

* fix buffer issues when reading with separators and limits; fix sysread

* Multiple fixes...
1. Fixed the #seek inheritance hierarchy mostly by no longer
conflating #seek and #sysseek as the same thing.
2. Rename PipeFileDescriptor to FIFOFileDescriptor since it
can also be used for special character files.
3. Fix #gets so the peek ahead for multi-byte characters is
properly respected; refill the buffer if we don't have enough
bytes for peek ahead.
4. Fix important error in #new_open_fd which was using the wrong
flag on an FD. When using fcntl on an FD, it is important to use
the FD_* flags. Was using O_CLOEXEC instead of FD_CLOEXEC and as
a result all child processes were hanging when their read pipe
was closed.
5. Handle EPIPE in #write correctly. Also raise an error if we
get an error instead of ignoring it.

* provide initial but broken C++ to Ruby conversion for IO.select

* Fixed FDSet to link.

This is a complex part of Rubinius and FFI where we need some primitive
code to interact with libc macros.

* converted until/end into begin/end until which fixed 4 specs

* add logic to load and detect posix_fadvise constants and enable feature detection

* add with_feature detection to skip these specs on platforms without posix_fadvise

* convert usage of posix_fadvise from C++ to Ruby code

* add select function signature; may need to change when we learn more about implementation

* continue fixup of IO.select; handle coercion and choosing max FD

* generate and save a string defining the struct timeval as a FFI struct

* add gettimeofday function for FFI access

* add FDSet::to_set to return FDSET for use by select

* finish fleshing out select support code; unfortunately SEGVs

* fix function signature for posix function #select

* fix returning pointer to descriptor_set

* pull FD_SETSIZE directly from the headers

* fix discovery of highest numbered FD; select still SEGVs

* return pointer to fd_set as a FFI pointer so we can use it from Ruby

* pass FFI Pointer to select; fixes SEGV but still does not work correctly

* fix SEGV by *correctly* getting the address of the data member

* fix #collect_set_fds; kind of works now

* Passes all but one select spec which is related to thread sleeping

Fixed +timeout+ handling so that a nil timeout is respected.
Prior to this we were allocating timeval structs every time and
it turns out that an empty struct and a nil value are not
equivalent in behavior.

Took the opportunity to refactor and DRY up some code that
validated IO.select arguments.

* modify this ugly hack a little to put struct in proper namespace

* add helper methods for raising EAGAIN and EAGAINWaitWritable

* disable the C++ version of #readpartial

* fix readpartial and read_nonblock specs; add new behavior for read_nonblock

* disable the C++ code for write_nonblock

* write the code to support write_nonblock and clean up some style issues

* comment out all primitives that are no longer in use

* stub out a call to #pos for 2 specs

* when reopening a FD flush buffer and seek to other FDs location; fix typo

* Support File#truncate and File#ftruncate correctly

Turns out that #truncate needed to be a class method, so I moved
it to a class method of FileDescriptor.

Also added support for creating a DirectoryFileDescriptor. It
merely inherits from FileDescriptor now. We will see if it needs
any new behavior or if default behavior needs to be curtailed.

* fix bug where kernel/common/io.rb was not set to nil after #foreach call

* fix bug in spec where incorrect args were passed to test #gets with limit

Tricky one to find. The original spec used #gets(1) to read one
char. Unfortunately, this arg format ends up setting a separator
to $/ so the logic ends up calling
EachReader#read_to_separator_with_limit instead of
EachReader#read_to_limit. By passing nil, as in #gets(nil, 1),
then we force the code paths to call read_to_limit.

* fix handling of multi-byte chars when reading with char limits

This fixes several specs shared by gets, foreach, and readline.
The EachReader class needs a good refactoring at this point
but for now I just want to commit working code.

* refactor IO::EachReader, dry it up, and improve code docs

* rescue ESPIPE from seek in #reopen when reopening a Pipe FD

* fix spec to properly handle the expected returns from #readlines

* move stdin/out/err redefinition under Rubinius::IOUtility namespace to avoid collisions

* fix indentation

* remove superfluous todo comments and dead/commented-out code

* fix indentation

* verify that non-blocking writes DO NOT BLOCK when a system buffer is full

* when a write returns EAGAIN or EINTR test to see if in non-blocking mode

Also took the opportunity to refactor fcntl F_GETFL and F_SETFL

* fix misuage of F_GETFL with F_GETFD

* first attempt at getting C-API updated to use Ruby obj over C++ obj

* fix several CAPI specs by correctly getting descriptor

* rename set_blocking method to clear_nonblock which makes more sense to me

* toggle the current thread sleep state when potentially blocking on read or select

* modify exception message to match MRI test expectations

* handle case where user calls IO.allocate and close

* allow multiple successive IO#close calls without error

* second attempt at fixing io=IO.allocate; io.close

* third try is the charm for IO.allocate; io.close

* multiple calls of close, close_read, and close_write should no longer raise IOError

* no longer raise IOError on multiple calls to close, close_read, or close_write

* refactor descriptor testing to make it cleaner

* remove duplicate check of ::read return value

* verify #read raises IOError when read interrupted by another thread closing socket

* always verify file is open after a read operation

* spec limit 0 behavior for each_lines for MRI 2.3 compatibility

* support raising ArgumentError given limit 0 for each_lines

* begin work on supporting Socket

* use rb_funcall properly; fixes SEGVs

* fix typo in private method name

* define method to raise EAGAINWaitWritable

* raise EAGAINWaitWritable when nonblocking write would block

* rescue exceptions when #find_type fails in @enclosing_module

This is really weird. The code calls #find_type in the scope
of the enclosing_module, but there could be a conflict with
that method name, numbers of args it takes, its purpose, etc.
So a failure here should be rescued and allow the followup
code to call #find_type on the FFI class.

This was discovered when the ffi-io branch failed to build
a native extension. The mkmf.rb gem was doing some logging
during the build which caused some code in IO::Select to
call IO::Select.class_eval on code to instantiate the
Timeval_t struct class. Creating a struct calls #find_type
but it was calling it from the scope of mkmf.rb which had
its own (completely different!) method named #find_type
defined. It threw an ArgumentError because the arity didn't
match, but it lead me to this logic error and fix.

* only allow seeking on 'file' file descriptors and not on fifo or other

During a call to IO.reopen a file descriptor was dup2'ed. Its
ftype changed from "file" to "fifo" and as we know a fifo cannot
seek. So we were blowing up when trying to set the offset ivar.
By only allowing seek on "file" FDs, we'll avoid the bug.

* remove spec for an IO implementation detail that no longer holds

As far as I can tell, there is no #buffer_empty? public method on
the IO class. We had one to test this behavior. Gone!

* remove unused and unnecessary public method

* remove temp spec file that was accidently committed earlier"

* emulate blocking read behavior using #read_nonblock

We had a failing spec where a thread blocked on read was
supposed to raise IOError when *another thread* closed
the IO. However, it didn't work. Turns out that the
low-level system read function blocks forever so the
closed IO/file descriptor didn't raise as expected.

The original C code got around this OS behavior by setting
special flags and sending signals to the blocked thread
to wake it up. This mechanism was only exposed to the
bytecode VM and wasn't available to the Ruby runtime so
I had to go this other direction. It's less than ideal
but if the long-term plan is to utilize libuv for
Rubinius IO then this is a good enough fix.

* change IO.setup to set fd accessor; fixes socket setup

I don't really like this solution very much. Ideally the
Socket classes would set @fd directly in their #initialize
methods via a call to FileDescriptor.choose_type(fd). Having
a +fd+ accessor on the IO class was intended only for
debugging purposes.

Let's go with this patch for now. If the larger refactoring
of the IO class to use a private IO::FileDescriptor class
is accepted then we can go back and change the Socket methods
to conform better and remove this accessor.

* replace calls to C++ IO obj with Ruby IO obj

* remove dead and commented-out code as part of cleanup

* spec out #ttyname behavior in Ruby

* add #ttyname function to FFI

* convert IO#ttyname from C++ to Ruby

* use Regexp to handle forward slash escaping in spec

* re-enable partial support for IO finalizer and autoclose

The IO finalizer is tricky since it also has to detect possible
modifications from C API calls. There will likely need to be
some rework done on the C API code itself to make this easier
to manage from the Ruby side since we are trying to get as much
as possible out of C/C++.

* A couple fixes for finding constants.

* Fixed accessing Stat in IO::FileDescriptor.

* make sure proper encoding set when using #read_nonblock

* first try to add finalizer support for C-extensions

* improve variable name

* if we fclose a FD then do not call close on it too

* use Ruby-based IO functions instead of C++ object

* remove unnecessary IO calls

* remove test file for dead C code

* remove dead C code for IO classes

* remove dead code

* make socket C funcs call ruby code from C; first try

* Fixed defining GC stubs for IO class.

* use calling frame when given else look it up

* Cleanup from stw branch merge.

* Revert "use Ruby-based IO functions instead of C++ object"

This reverts commit ca75509.

Re-introduced IO::open_with_cloexec to clean up this.

* Removed test_io.hpp incorrectly merged.

* Fixed IO::FileDescriptor#finalizer definition.

* Switched fork/exec lock to SpinLock.

Since a SpinLock is a simple integer on which CAS operations are performed,
there is no way to go afoul of 'ownership' during fork(). This appears to
solve a spordic issue where the child was not able to reset the fork_exec_lock_
inherited from the parent process.

* Properly assign rio_stream GC root.

* Use built-in finalizer protocol for IO-related objects.

* use files instead of pipes to test finalizers

* move fd instance alloc to IO.setup

* fix method signature; remove debug print

* call method on self via send

Before the code was looking up 'self' by getting the current
call frame and pulling self from it. This was returning a
different object than expected. So now we just call #send
against the current object; this works consistently.

* cast unsigned vals back to signed to fix compiler warnings

* modify order of operations via parens to satisfy compiler

* hack to prevent closing rio twice

* Removed .tap generating structs.

* Removed double finalization bandaid.

We need to ensure this is fixed now.

* Re-added Rubinius define_finalizer extension.

* Fixed finalizer spec to clean up temporary.

* ignore NotImplementedError for #fadvise on platforms that do not support that call

* bump ruby compatibility to 2.3.0

* rename spec file so ci run matches 2.3.0 ruby compat

* verify that infinitely looping subprocesses can be force closed

* remove special case EPIPE error handling so all write errors bubble up

* clarify intent of spec

* make sure we test for new transcoding options

* prepare to handle passing more encoding options in IO.open/File.open

* stub in an attempt at hooking up newline transcoders

* stub in attempt at accessing transcoders

* fix regex so it matches newline transcoders

* Revert "stub in attempt at accessing transcoders"

This reverts commit 6455dc7.

* Revert "stub in an attempt at hooking up newline transcoders"

This reverts commit 39d255f.

* Revert "make sure we test for new transcoding options"

This reverts commit 679a247.

* fix encoding_options handling to pass specs

* Guard calling fclose() on NULL rio-f.
chuckremes authored and brixen committed Jul 11, 2016
1 parent f03286d commit a8f23c1
Showing 34 changed files with 2,234 additions and 1,973 deletions.
27 changes: 26 additions & 1 deletion core/errno.rb
Original file line number Diff line number Diff line change
@@ -9,9 +9,34 @@ module Errno
# Unlike rb_sys_fail(), handle does not raise an exception if errno is 0.

def self.handle(additional = nil)
err = FFI::Platform::POSIX.errno
err = errno
return if err == 0

raise SystemCallError.new(additional, err)
end

def self.errno
FFI.errno
end

def self.eql?(code)
FFI.errno == code
end

def self.raise_waitreadable(message=nil)
raise IO::EAGAINWaitReadable, message
end

def self.raise_waitwritable(message=nil)
raise IO::EAGAINWaitWritable, message
end

def self.raise_eagain(message=nil)
raise_errno(message, Errno::EAGAIN::Errno)
end

def self.raise_errno(message, errno)
raise SystemCallError.new(message, errno)
end
private :raise_errno
end
6 changes: 5 additions & 1 deletion core/ffi.rb
Original file line number Diff line number Diff line change
@@ -72,6 +72,10 @@ def errno
FFI::Platform::POSIX.errno
end

# Convenience method for determining if a function call succeeded or failed
def call_failed?(return_code)
return_code == -1
end
end

# Represents a C enum.
@@ -318,7 +322,7 @@ def self.layout(*spec)
element_size = type_size = f.size
else
if @enclosing_module
type_code = @enclosing_module.find_type(f)
type_code = @enclosing_module.find_type(f) rescue nil
end

type_code ||= FFI.find_type(f)
13 changes: 7 additions & 6 deletions core/file.rb
Original file line number Diff line number Diff line change
@@ -250,7 +250,7 @@ def self.chown(owner, group, *paths)

def chmod(mode)
mode = Rubinius::Type.coerce_to(mode, Integer, :to_int)
n = POSIX.fchmod @descriptor, clamp_short(mode)
n = POSIX.fchmod descriptor, clamp_short(mode)
Errno.handle if n == -1
n
end
@@ -268,7 +268,7 @@ def chown(owner, group)
group = -1
end

n = POSIX.fchown @descriptor, owner, group
n = POSIX.fchown descriptor, owner, group
Errno.handle if n == -1
n
end
@@ -1018,7 +1018,7 @@ def self.truncate(path, length)

length = Rubinius::Type.coerce_to length, Integer, :to_int

prim_truncate(path, length)
FileDescriptor.truncate(path, length)
end

##
@@ -1189,6 +1189,7 @@ def initialize(path_or_fd, mode=undefined, perm=undefined, options=undefined)
Errno.handle path if fd < 0

@path = path

super(fd, mode, options)
end
end
@@ -1230,7 +1231,7 @@ def reopen(other, mode = 'r+')
def flock(const)
const = Rubinius::Type.coerce_to const, Integer, :to_int

result = POSIX.flock @descriptor, const
result = POSIX.flock descriptor, const

return false if result == -1
result
@@ -1241,7 +1242,7 @@ def lstat
end

def stat
Stat.fstat @descriptor
Stat.fstat descriptor
end

alias_method :to_path, :path
@@ -1254,7 +1255,7 @@ def truncate(length)

flush
reset_buffering
prim_ftruncate(length)
@fd.ftruncate(length)
end

def inspect
Loading

0 comments on commit a8f23c1

Please sign in to comment.