[GR-18163] Implement changes of ruby-3.0 to IO#wait (#2953)

andrykonchin · andrykonchin · commit 365a678e9940 · 2023-03-31T17:03:16.000Z
PullRequest: truffleruby/3724
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -120,6 +120,7 @@ Compatibility:
 * Upgrading `UNICODE` version to 13.0.0 and `EMOJI` version to 13.1 (#2733, @horakivo).
 * Add `rb_io_maybe_wait_readable`, `rb_io_maybe_wait_writable` and `rb_io_maybe_wait` functions (#2733, @andrykonchin).
 * `StringIO#set_encoding` should coerce the argument to an Encoding (#2954, @eregon).
+* Implement changes of Ruby 3.0 to `IO#wait` (#2953, @larskanis).
 
 Performance:
 
diff --git a/lib/truffle/io/wait.rb b/lib/truffle/io/wait.rb
@@ -1,3 +1,5 @@
+# truffleruby_primitives: true
+
 # Copyright (c) 2017, 2023 Oracle and/or its affiliates. All rights reserved. This
 # code is released under a tri EPL/GPL/LGPL license. You can use it,
 # redistribute it and/or modify it under the terms of the:
@@ -14,17 +16,77 @@ def nread
 
   def ready?
     ensure_open_and_readable
-    Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLIN, 0)
+    Truffle::IOOperations.poll(self, IO::READABLE, 0) > 0
   end
 
   def wait_readable(timeout = nil)
     ensure_open_and_readable
-    Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLIN, timeout) ? self : nil
+    Truffle::IOOperations.poll(self, IO::READABLE, timeout) > 0 ? self : nil
   end
-  alias_method :wait, :wait_readable
 
   def wait_writable(timeout = nil)
     ensure_open_and_writable
-    Truffle::IOOperations.poll(self, Truffle::IOOperations::POLLOUT, timeout) ? self : nil
+    Truffle::IOOperations.poll(self, IO::WRITABLE, timeout) > 0 ? self : nil
+  end
+
+  def wait_priority(timeout = nil)
+    ensure_open_and_readable
+    Truffle::IOOperations.poll(self, IO::PRIORITY, timeout) > 0 ? self : nil
+  end
+
+
+  # call-seq:
+  #   io.wait(events, timeout) -> event mask, false or nil
+  #   io.wait(timeout = nil, mode = :read) -> self, true, or false
+  #
+  # Waits until the IO becomes ready for the specified events and returns the
+  # subset of events that become ready, or a falsy value when times out.
+  #
+  # The events can be a bit mask of +IO::READABLE+, +IO::WRITABLE+ or
+  # +IO::PRIORITY+.
+  #
+  # Returns a truthy value immediately when buffered data is available.
+  #
+  # Optional parameter +mode+ is one of +:read+, +:write+, or
+  # +:read_write+.
+  def wait(*args)
+    ensure_open
+
+    if args.size != 2 || Primitive.object_kind_of?(args[0], Symbol) || Primitive.object_kind_of?(args[1], Symbol)
+      # Slow/messy path:
+      timeout = :undef
+      events = 0
+      args.each do |arg|
+        if Primitive.object_kind_of?(arg, Symbol)
+          events |= case arg
+                    when :r, :read, :readable then IO::READABLE
+                    when :w, :write, :writable then IO::WRITABLE
+                    when :rw, :read_write, :readable_writable then IO::READABLE | IO::WRITABLE
+                    else
+                      raise ArgumentError, "unsupported mode: #{arg}"
+                    end
+
+        elsif timeout == :undef
+          timeout = arg
+        else
+          raise ArgumentError, 'timeout given more than once'
+        end
+      end
+
+      timeout = nil if timeout == :undef
+
+      events = IO::READABLE if events == 0
+
+      res = Truffle::IOOperations.poll(self, events, timeout)
+      res == 0 ? nil : self
+    else
+      # args.size == 2 and neither are symbols
+      # This is the fast path and the new interface:
+      events, timeout = *args
+      raise ArgumentError, 'Events must be positive integer!' if events <= 0
+      res = Truffle::IOOperations.poll(self, events, timeout)
+      # return events as bit mask
+      res == 0 ? nil : res
+    end
   end
 end
diff --git a/spec/ruby/library/io-wait/fixtures/classes.rb b/spec/ruby/library/io-wait/fixtures/classes.rb
@@ -0,0 +1,12 @@
+module IOWaitSpec
+  def self.exhaust_write_buffer(io)
+    written = 0
+    buf = " " * 4096
+
+    begin
+      written += io.write_nonblock(buf)
+    rescue Errno::EAGAIN, Errno::EWOULDBLOCK
+      return written
+    end while true
+  end
+end
diff --git a/spec/ruby/library/io-wait/wait_spec.rb b/spec/ruby/library/io-wait/wait_spec.rb
@@ -0,0 +1,147 @@
+require_relative '../../spec_helper'
+require_relative 'fixtures/classes'
+
+ruby_version_is ''...'3.2' do
+  require 'io/wait'
+end
+
+describe "IO#wait" do
+  before :each do
+    @io = File.new(__FILE__ )
+
+    if /mswin|mingw/ =~ RUBY_PLATFORM
+      @r, @w = Socket.pair(Socket::AF_INET, Socket::SOCK_STREAM, 0)
+    else
+      @r, @w = IO.pipe
+    end
+  end
+
+  after :each do
+    @io.close unless @io.closed?
+
+    @r.close unless @r.closed?
+    @w.close unless @w.closed?
+  end
+
+  ruby_version_is "3.0" do
+    context "[events, timeout] passed" do
+      ruby_version_is "3.0"..."3.2" do
+        it "returns self when the READABLE event is ready during the timeout" do
+          @w.write('data to read')
+          @r.wait(IO::READABLE, 2).should.equal?(@r)
+        end
+
+        it "returns self when the WRITABLE event is ready during the timeout" do
+          @w.wait(IO::WRITABLE, 0).should.equal?(@w)
+        end
+      end
+
+      ruby_version_is "3.2" do
+        it "returns events mask when the READABLE event is ready during the timeout" do
+          @w.write('data to read')
+          @r.wait(IO::READABLE, 2).should == IO::READABLE
+        end
+
+        it "returns events mask when the WRITABLE event is ready during the timeout" do
+          @w.wait(IO::WRITABLE, 0).should == IO::WRITABLE
+        end
+      end
+
+      ruby_version_is "3.0" do
+        it "waits for the READABLE event to be ready" do
+          queue = Queue.new
+          thread = Thread.new { queue.pop; sleep 1; @w.write('data to read') };
+
+          queue.push('signal');
+          @r.wait(IO::READABLE, 2).should_not == nil
+
+          thread.join
+        end
+
+        it "waits for the WRITABLE event to be ready" do
+          written_bytes = IOWaitSpec.exhaust_write_buffer(@w)
+
+          queue = Queue.new
+          thread = Thread.new { queue.pop; sleep 1; @r.read(written_bytes) };
+
+          queue.push('signal');
+          @w.wait(IO::WRITABLE, 2).should_not == nil
+
+          thread.join
+        end
+
+        it "returns nil when the READABLE event is not ready during the timeout" do
+          @w.wait(IO::READABLE, 0).should == nil
+        end
+
+        it "returns nil when the WRITABLE event is not ready during the timeout" do
+          IOWaitSpec.exhaust_write_buffer(@w)
+          @w.wait(IO::WRITABLE, 0).should == nil
+        end
+
+        it "raises IOError when io is closed (closed stream (IOError))" do
+          @io.close
+          -> { @io.wait(IO::READABLE, 0) }.should raise_error(IOError, "closed stream")
+        end
+
+        ruby_version_is "3.2" do
+          it "raises ArgumentError when events is not positive" do
+            -> { @w.wait(0, 0) }.should raise_error(ArgumentError, "Events must be positive integer!")
+            -> { @w.wait(-1, 0) }.should raise_error(ArgumentError, "Events must be positive integer!")
+          end
+        end
+      end
+    end
+  end
+
+  context "[timeout, mode] passed" do
+    it "accepts :r, :read, :readable mode to check READABLE event" do
+      @io.wait(0, :r).should == @io
+      @io.wait(0, :read).should == @io
+      @io.wait(0, :readable).should == @io
+    end
+
+    it "accepts :w, :write, :writable mode to check WRITABLE event" do
+      @io.wait(0, :w).should == @io
+      @io.wait(0, :write).should == @io
+      @io.wait(0, :writable).should == @io
+    end
+
+    it "accepts :rw, :read_write, :readable_writable mode to check READABLE and WRITABLE events" do
+      @io.wait(0, :rw).should == @io
+      @io.wait(0, :read_write).should == @io
+      @io.wait(0, :readable_writable).should == @io
+    end
+
+    it "accepts a list of modes" do
+       @io.wait(0, :r, :w, :rw).should == @io
+    end
+
+    # It works at least since 2.7 but by some reason may fail on 3.1
+    ruby_version_is "3.2" do
+      it "accepts timeout and mode in any order" do
+        @io.wait(0, :r).should == @io
+        @io.wait(:r, 0).should == @io
+        @io.wait(:r, 0, :w).should == @io
+      end
+    end
+
+    it "raises ArgumentError when passed wrong Symbol value as mode argument" do
+      -> { @io.wait(0, :wrong) }.should raise_error(ArgumentError, "unsupported mode: wrong")
+    end
+
+    # It works since 3.0 but by some reason may fail on 3.1
+    ruby_version_is "3.2" do
+      it "raises ArgumentError when several Integer arguments passed" do
+        -> { @w.wait(0, 10, :r) }.should raise_error(ArgumentError, "timeout given more than once")
+      end
+    end
+
+    ruby_version_is "3.2" do
+      it "raises IOError when io is closed (closed stream (IOError))" do
+        @io.close
+        -> { @io.wait(0, :r) }.should raise_error(IOError, "closed stream")
+      end
+    end
+  end
+end
diff --git a/spec/tags/library/io-wait/wait_tags.txt b/spec/tags/library/io-wait/wait_tags.txt
@@ -0,0 +1,3 @@
+fails:IO#wait [events, timeout] passed returns self when the READABLE event is ready during the timeout
+fails:IO#wait [events, timeout] passed returns self when the WRITABLE event is ready during the timeout
+fails:IO#wait [events, timeout] passed waits for the READABLE event to be ready
diff --git a/src/main/c/truffleposix/truffleposix.c b/src/main/c/truffleposix/truffleposix.c
@@ -117,7 +117,7 @@ int truffleposix_poll(int fd, int events, int timeout_ms) {
   fds.fd = fd;
   fds.events = events;
 
-  return poll(&fds, 1, timeout_ms);
+  return poll(&fds, 1, timeout_ms) >= 0 ? fds.revents : -1;
 }
 
 int truffleposix_select(int nread, int *readfds, int nwrite, int *writefds,
diff --git a/src/main/ruby/truffleruby/core/truffle/io_operations.rb b/src/main/ruby/truffleruby/core/truffle/io_operations.rb
@@ -205,9 +205,9 @@ def self.select(readables, readable_ios, writables, writable_ios, errorables, er
 
     end
 
-    # This method will return a true if poll returned without error
-    # with an event within the timeout, false if the timeout expired,
-    # or raises an exception for an errno.
+    # This method will return an event mask if poll returned without error.
+    # The event mask is > 0 when an event occurred within the timeout, 0 if the timeout expired.
+    # Raises an exception for an errno.
     def self.poll(io, event_mask, timeout)
       if (event_mask & POLLIN) != 0
         return 1 unless io.__send__(:buffer_empty?)
@@ -257,7 +257,7 @@ def self.poll(io, event_mask, timeout)
               Errno.handle_errno(errno)
             end
           else
-            primitive_result > 0
+            primitive_result
           end
       end while result == :retry
 
diff --git a/test/mri/excludes/TestIOWait.rb b/test/mri/excludes/TestIOWait.rb
@@ -1,12 +1,4 @@
-exclude :test_nread, "needs investigation"
-exclude :test_nread_buffered, "needs investigation"
-exclude :test_wait_buffered, "needs investigation"
-exclude :test_wait_readable_buffered, "needs investigation"
-exclude :test_wait_readwrite, "needs investigation"
-exclude :test_wait_readwrite_timeout, "needs investigation"
-exclude :test_nread, "needs investigation"
-exclude :test_nread_buffered, "needs investigation"
-exclude :test_wait_buffered, "needs investigation"
-exclude :test_wait_readable_buffered, "needs investigation"
-exclude :test_wait_readwrite, "needs investigation"
-exclude :test_wait_readwrite_timeout, "needs investigation"
+exclude :test_nread, "buffered reads are not implemented"
+exclude :test_nread_buffered, "buffered reads are not implemented"
+exclude :test_wait_buffered, "buffered reads are not implemented"
+exclude :test_wait_readable_buffered, "buffered reads are not implemented"
diff --git a/test/mri/tests/io/wait/test_io_wait.rb b/test/mri/tests/io/wait/test_io_wait.rb
@@ -3,10 +3,9 @@
 require 'test/unit'
 require 'timeout'
 require 'socket'
-begin
-  require 'io/wait'
-rescue LoadError
-end
+
+# For `IO#ready?` and `IO#nread`:
+require 'io/wait'
 
 class TestIOWait < Test::Unit::TestCase
 
@@ -50,6 +49,7 @@ def test_buffered_ready?
   end
 
   def test_wait
+    omit 'unstable on MinGW' if /mingw/ =~ RUBY_PLATFORM
     assert_nil @r.wait(0)
     @w.syswrite "."
     sleep 0.1
@@ -161,6 +161,34 @@ def test_wait_readwrite_timeout
     assert_equal @w, @w.wait(0.01, :read_write)
   end
 
+  def test_wait_mask_writable
+    omit("Missing IO::WRITABLE!") unless IO.const_defined?(:WRITABLE)
+    assert_equal IO::WRITABLE, @w.wait(IO::WRITABLE, 0)
+  end
+
+  def test_wait_mask_readable
+    omit("Missing IO::READABLE!") unless IO.const_defined?(:READABLE)
+    @w.write("Hello World\n" * 3)
+    assert_equal IO::READABLE, @r.wait(IO::READABLE, 0)
+
+    @r.gets
+    assert_equal IO::READABLE, @r.wait(IO::READABLE, 0)
+  end
+
+  def test_wait_mask_zero
+    omit("Missing IO::WRITABLE!") unless IO.const_defined?(:WRITABLE)
+    assert_raise(ArgumentError) do
+      @w.wait(0, 0)
+    end
+  end
+
+  def test_wait_mask_negative
+    omit("Missing IO::WRITABLE!") unless IO.const_defined?(:WRITABLE)
+    assert_raise(ArgumentError) do
+      @w.wait(-6, 0)
+    end
+  end
+
 private
 
   def fill_pipe

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+fails:IO#wait [events, timeout] passed returns self when the READABLE event is ready during the timeout`
	`2`	`+fails:IO#wait [events, timeout] passed returns self when the WRITABLE event is ready during the timeout`
	`3`	`+fails:IO#wait [events, timeout] passed waits for the READABLE event to be ready`
Original file line number	Diff line number	Diff line change
`@@ -117,7 +117,7 @@ int truffleposix_poll(int fd, int events, int timeout_ms) {`
`117`	`117`	`fds.fd = fd;`
`118`	`118`	`fds.events = events;`
`119`	`119`
`120`		`- return poll(&fds, 1, timeout_ms);`
	`120`	`+ return poll(&fds, 1, timeout_ms) >= 0 ? fds.revents : -1;`
`121`	`121`	`}`
`122`	`122`
`123`	`123`	`int truffleposix_select(int nread, int readfds, int nwrite, int writefds,`