Skip to content

Commit

Permalink
Reimplement IO.select with poll(2)
Browse files Browse the repository at this point in the history
* This avoids the issue of file descriptors >= 1024 that FD_ZERO does not handle.
* Fixes #3201
* Skip TestIO#test_select_exceptfds on macOS, macOS's poll(2) is buggy with TCP MSG_OOB or macOS does not really support MSG_OOB:
  For a fd to which out-of-band data has been sent,
  poll() with POLLIN|POLLPRI returns POLLRDNORM|POLLPRI|POLLIN on Linux (correct) but POLLRDNORM|POLLIN on macOS (bug).
  poll() with just POLLPRI works fine on Linux but hangs on macOS (bug).
  It seems a bug of macOS poll() not handling TCP MSG_OOB.
  The man page of poll() on macOS talks about: "The distinction between normal, priority, and high-priority data
  is specific to particular file types or devices." but gives no details, it seems a mess.
  MSG_OOB is poorly supported across platforms anyway and extremely rarely if ever used.
  • Loading branch information
eregon committed Aug 30, 2023
1 parent 77b8125 commit 3181488
Show file tree
Hide file tree
Showing 13 changed files with 108 additions and 97 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Bug fixes:
* Fix `super` method lookup for unbounded attached methods (#3131, @itarato).
* Fix `Module#define_method(name, Method)` to respect `module_function` visibility (#3181, @andrykonchin).
* Fix stack overflow with `Kernel.require` and `zeitwerk` (#3224, @eregon).
* Reimplement `IO.select` with `poll(2)` to support file descriptors >= 1024 (#3201, @eregon).

Compatibility:

Expand Down
15 changes: 13 additions & 2 deletions spec/ruby/core/io/select_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,26 @@
end
end

it "returns supplied objects correctly even when monitoring the same object in different arrays" do
filename = tmp("IO_select_pipe_file") + $$.to_s
it "returns supplied objects correctly when monitoring the same object in different arrays" do
filename = tmp("IO_select_pipe_file")
io = File.open(filename, 'w+')
result = IO.select [io], [io], nil, 0
result.should == [[io], [io], []]
io.close
rm_r filename
end

it "returns the pipe read end in read set if the pipe write end is closed concurrently" do
main = Thread.current
t = Thread.new {
Thread.pass until main.stop?
@wr.close
}
IO.select([@rd]).should == [[@rd], [], []]
ensure
t.join
end

it "invokes to_io on supplied objects that are not IO and returns the supplied objects" do
# make some data available
@wr.write("foobar")
Expand Down
10 changes: 5 additions & 5 deletions spec/truffle/io/read_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@
w.write "b"
w.flush

select_ignoring_iobuffer([r], 1_000_000).should == [r]
select_ignoring_iobuffer([r], 100).should == [r]
r.read(1).should == "a"

select_ignoring_iobuffer([r], 1_000_000).should == [r]
select_ignoring_iobuffer([r], 1000).should == [r]
r.read(1).should == "b"
end

Expand All @@ -42,17 +42,17 @@
@w.flush

@r.gets.should == "a\n"
select_ignoring_iobuffer([@r], 100_000).should == nil
select_ignoring_iobuffer([@r], 100).should == nil
end

def select_ignoring_iobuffer(ios, timeout_us)
def select_ignoring_iobuffer(ios, timeout_ms)
return IO.select(ios)[0] unless defined?(::TruffleRuby)

result = Truffle::IOOperations.select(
ios, ios,
[], [],
[], [],
timeout_us, timeout_us)
timeout_ms)
result and result[0]
end
end
49 changes: 1 addition & 48 deletions src/main/c/truffleposix/truffleposix.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ SUCH DAMAGE.
#include <sys/file.h>
#include <sys/resource.h>
#include <sys/stat.h>
#include <sys/select.h>
#include <sys/time.h>
#include <sys/types.h>
#include <sys/wait.h>
Expand Down Expand Up @@ -91,27 +90,7 @@ struct truffleposix_stat {

static void copy_stat(struct stat *stat, struct truffleposix_stat* buffer);

static void init_fd_set(fd_set *set, int nfds, int *fds, int *maxfd) {
FD_ZERO(set);
for (int i = 0; i < nfds; i++) {
int fd = fds[i];
FD_SET(fd, set);
if (fd > *maxfd) {
*maxfd = fd;
}
}
}

static void mark_ready_from_set(fd_set *set, int nfds, int *fds) {
for (int i = 0; i < nfds; i++) {
int fd = fds[i];
if (!FD_ISSET(fd, set)) {
fds[i] = -1;
}
}
}

int truffleposix_poll(int fd, int events, int timeout_ms) {
int truffleposix_poll_single_fd(int fd, int events, int timeout_ms) {
struct pollfd fds;

fds.fd = fd;
Expand All @@ -120,32 +99,6 @@ int truffleposix_poll(int fd, int events, int timeout_ms) {
return poll(&fds, 1, timeout_ms) >= 0 ? fds.revents : -1;
}

int truffleposix_select(int nread, int *readfds, int nwrite, int *writefds,
int nexcept, int *exceptfds, long timeout_us) {
struct timeval timeout;
struct timeval *timeout_ptr = NULL;

if (timeout_us >= 0) {
timeout.tv_sec = (timeout_us / 1000000);
timeout.tv_usec = (timeout_us % 1000000);
timeout_ptr = &timeout;
}

int maxfd = 0;
fd_set readset, writeset, exceptset;
init_fd_set(&readset, nread, readfds, &maxfd);
init_fd_set(&writeset, nwrite, writefds, &maxfd);
init_fd_set(&exceptset, nexcept, exceptfds, &maxfd);

int ret = select(maxfd+1, &readset, &writeset, &exceptset, timeout_ptr);
if (ret > 0) {
mark_ready_from_set(&readset, nread, readfds);
mark_ready_from_set(&writeset, nwrite, writefds);
mark_ready_from_set(&exceptset, nexcept, exceptfds);
}
return ret;
}

int truffleposix_utimes(const char *filename, long atime_sec, int atime_nsec,
long mtime_sec, int mtime_nsec) {
struct timespec timespecs[2];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,12 @@ public static void load(NativeConfiguration configuration, RubyContext context)
configuration.config("platform.poll.POLLIN", 1);
configuration.config("platform.poll.POLLPRI", 2);
configuration.config("platform.poll.POLLOUT", 4);
configuration.config("platform.poll.POLLERR", 8);
configuration.config("platform.poll.POLLHUP", 16);
configuration.config("platform.poll.POLLRDNORM", 64);
configuration.config("platform.poll.POLLRDBAND", 128);
configuration.config("platform.poll.POLLWRNORM", 4);
configuration.config("platform.poll.POLLWRBAND", 256);
configuration.config("platform.socket.AF_APPLETALK", 16);
configuration.config("platform.socket.PF_APPLETALK", 16);
configuration.config("platform.socket.AF_INET", 2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,12 @@ public static void load(NativeConfiguration configuration, RubyContext context)
configuration.config("platform.poll.POLLIN", 1);
configuration.config("platform.poll.POLLPRI", 2);
configuration.config("platform.poll.POLLOUT", 4);
configuration.config("platform.poll.POLLERR", 8);
configuration.config("platform.poll.POLLHUP", 16);
configuration.config("platform.poll.POLLRDNORM", 64);
configuration.config("platform.poll.POLLRDBAND", 128);
configuration.config("platform.poll.POLLWRNORM", 4);
configuration.config("platform.poll.POLLWRBAND", 256);
configuration.config("platform.socket.AF_APPLETALK", 16);
configuration.config("platform.socket.PF_APPLETALK", 16);
configuration.config("platform.socket.AF_INET", 2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,12 @@ public static void load(NativeConfiguration configuration, RubyContext context)
configuration.config("platform.poll.POLLIN", 1);
configuration.config("platform.poll.POLLPRI", 2);
configuration.config("platform.poll.POLLOUT", 4);
configuration.config("platform.poll.POLLERR", 8);
configuration.config("platform.poll.POLLHUP", 16);
configuration.config("platform.poll.POLLRDNORM", 64);
configuration.config("platform.poll.POLLRDBAND", 128);
configuration.config("platform.poll.POLLWRNORM", 256);
configuration.config("platform.poll.POLLWRBAND", 512);
configuration.config("platform.socket.AF_APPLETALK", 5);
configuration.config("platform.socket.PF_APPLETALK", 5);
configuration.config("platform.socket.AF_AX25", 3);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,12 @@ public static void load(NativeConfiguration configuration, RubyContext context)
configuration.config("platform.poll.POLLIN", 1);
configuration.config("platform.poll.POLLPRI", 2);
configuration.config("platform.poll.POLLOUT", 4);
configuration.config("platform.poll.POLLERR", 8);
configuration.config("platform.poll.POLLHUP", 16);
configuration.config("platform.poll.POLLRDNORM", 64);
configuration.config("platform.poll.POLLRDBAND", 128);
configuration.config("platform.poll.POLLWRNORM", 256);
configuration.config("platform.poll.POLLWRBAND", 512);
configuration.config("platform.socket.AF_APPLETALK", 5);
configuration.config("platform.socket.PF_APPLETALK", 5);
configuration.config("platform.socket.AF_AX25", 3);
Expand Down
8 changes: 3 additions & 5 deletions src/main/ruby/truffleruby/core/io.rb
Original file line number Diff line number Diff line change
Expand Up @@ -735,10 +735,8 @@ def self.select(readables = nil, writables = nil, errorables = nil, timeout = ni

raise ArgumentError, 'timeout must be positive' if timeout < 0

# Microseconds, rounded down
timeout = remaining_timeout = Integer(timeout * 1_000_000)
else
remaining_timeout = -1
# Milliseconds, rounded down
timeout = Integer(timeout * 1_000)
end

if readables
Expand Down Expand Up @@ -782,7 +780,7 @@ def self.select(readables = nil, writables = nil, errorables = nil, timeout = ni
readables, readable_ios,
writables, writable_ios,
errorables, errorable_ios,
timeout, remaining_timeout)
timeout)
end

##
Expand Down
10 changes: 5 additions & 5 deletions src/main/ruby/truffleruby/core/posix.rb
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,9 @@ def self.attach_function_eagerly(native_name, argument_types, return_type,
attach_function :open, [:string, :int, varargs(:mode_t)], :int
attach_function :opendir, [:string], :pointer
attach_function :pipe, [:pointer], :int
attach_function :truffleposix_poll, [:pointer, :long, :int], :int, LIBTRUFFLEPOSIX, true
# blocking=false for both poll because the timeout needs to be decreased on EINTR
attach_function :truffleposix_poll_single_fd, [:pointer, :long, :int], :int, LIBTRUFFLEPOSIX
attach_function :poll, [:pointer, :long, :int], :int
attach_function :read, [:int, :pointer, :size_t], :ssize_t, LIBC, true
attach_function :readlink, [:string, :pointer, :size_t], :ssize_t
attach_function :realpath, [:string, :pointer], :pointer
Expand All @@ -224,8 +226,6 @@ def self.attach_function_eagerly(native_name, argument_types, return_type,
attach_function :truffleposix_rewinddir, [:pointer], :void, LIBTRUFFLEPOSIX
attach_function :rmdir, [:string], :int
attach_function :seekdir, [:pointer, :long], :void
select_args = [:int, :pointer, :int, :pointer, :int, :pointer, :long]
attach_function :truffleposix_select, select_args, :int, LIBTRUFFLEPOSIX
attach_function :truffleposix_stat, [:string, :pointer], :int, LIBTRUFFLEPOSIX
attach_function :truffleposix_stat_mode, [:string], :mode_t, LIBTRUFFLEPOSIX
attach_function :truffleposix_stat_size, [:string], :long, LIBTRUFFLEPOSIX
Expand All @@ -240,8 +240,8 @@ def self.attach_function_eagerly(native_name, argument_types, return_type,
Truffle::Boot.delay do
if NATIVE
# We should capture the non-lazy method
attach_function_eagerly :truffleposix_select, select_args, :int, LIBTRUFFLEPOSIX, false, :truffleposix_select, self
SELECT = method(:truffleposix_select)
attach_function_eagerly :poll, [:pointer, :long, :int], :int, LIBC, false, :poll, self
POLL = method(:poll)
end
end

Expand Down
Loading

0 comments on commit 3181488

Please sign in to comment.