jruby · Jan 2, 2016 · Jan 3, 2016 · Jan 5, 2016 · Jan 7, 2016 · Jan 7, 2016
Showing with 104 additions and 76 deletions.

+11 −4 core/src/main/java/org/jruby/RubyIO.java

+74 −60 core/src/main/java/org/jruby/util/io/SelectBlob.java

+19 −12 test/jruby/test_io.rb
diff --git a/core/src/main/java/org/jruby/RubyIO.java b/core/src/main/java/org/jruby/RubyIO.java
@@ -3314,7 +3314,6 @@ public static RubyIO convertToIO(ThreadContext context, IRubyObject obj) {
     @JRubyMethod(name = "select", required = 1, optional = 3, meta = true)
     public static IRubyObject select(ThreadContext context, IRubyObject recv, IRubyObject[] argv) {
         IRubyObject read, write, except, _timeout;
-        Long timeout;
         read = write = except = _timeout = context.nil;
 
         switch (argv.length) {
@@ -3327,13 +3326,21 @@ public static IRubyObject select(ThreadContext context, IRubyObject recv, IRubyO
             case 1:
                 read = argv[0];
         }
+        final Long timeout;
         if (_timeout.isNil()) {
             timeout = null;
         }
         else {
-            double tmp = _timeout.convertToFloat().getDoubleValue();
-            if (tmp < 0) throw context.runtime.newArgumentError("negative timeout");
-            timeout = (long)(tmp * 1000); // ms
+            try { // MRI calls to_f even if not respond_to? (or respond_to_missing?) :to_f
+                _timeout = _timeout.callMethod(context, "to_f");
+            }
+            catch (RaiseException e) {
+                TypeConverter.handleUncoercibleObject(true, _timeout, context.runtime.getFloat());
+                throw e; // won't happen
+            }
+            final double t = _timeout.convertToFloat().getDoubleValue();
+            if ( t < 0 ) throw context.runtime.newArgumentError("negative timeout");
+            timeout = (long) (t * 1000); // ms
         }
 
         SelectExecutor args = new SelectExecutor(read, write, except, timeout);

diff --git a/core/src/main/java/org/jruby/util/io/SelectBlob.java b/core/src/main/java/org/jruby/util/io/SelectBlob.java
@@ -11,7 +11,7 @@
  * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
  * implied. See the License for the specific language governing
  * rights and limitations under the License.
- * 
+ *
  * Alternatively, the contents of this file may be used under the terms of
  * either of the GNU General Public License Version 2 or later (the "GPL"),
  * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
@@ -28,7 +28,7 @@
 
 import org.jruby.Ruby;
 import org.jruby.RubyArray;
-import org.jruby.RubyFixnum;
+import org.jruby.RubyInteger;
 import org.jruby.RubyFloat;
 import org.jruby.RubyIO;
 import org.jruby.RubyThread;
@@ -44,11 +44,12 @@
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
+import org.jruby.exceptions.RaiseException;
 
 /**
  * This is a reimplementation of MRI's IO#select logic. It has been rewritten
  * from an earlier version in JRuby to improve performance and readability.
- * 
+ *
  * This version avoids allocating a selector or any data structures to hold
  * data about the channels/IOs being selected unless absolutely necessary. It
  * also uses simple boolean arrays to track characteristics like whether an IO
@@ -70,13 +71,13 @@ public IRubyObject goForIt(ThreadContext context, Ruby runtime, IRubyObject[] ar
                 checkArrayType(runtime, args[2]);
                 // Java's select doesn't do anything about this, so we leave it be.
             }
-            boolean has_timeout = args.length > 3 && !args[3].isNil();
-            long timeout = !has_timeout ? 0 : getTimeoutFromArg(args[3], runtime);
-            
+            final boolean has_timeout = args.length > 3 && !args[3].isNil();
+            final long timeout = !has_timeout ? 0 : convertTimeout(context, args[3]);
+
             if (timeout < 0) {
                 throw runtime.newArgumentError("time interval must be positive");
             }
-            
+
             // If all streams are nil, just sleep the specified time (JRUBY-4699)
             if (args[0].isNil() && args[1].isNil() && args[2].isNil()) {
                 RubyThread thread = context.getThread();
@@ -98,7 +99,7 @@ public IRubyObject goForIt(ThreadContext context, Ruby runtime, IRubyObject[] ar
                 processPendingAndUnselectable();
                 tidyUp();
             }
-            
+
             if (readResults == null && writeResults == null && errorResults == null) {
                 return runtime.getNil();
             }
@@ -136,7 +137,7 @@ private void processReads(Ruby runtime, IRubyObject[] args, ThreadContext contex
                 for (int i = 0; i < readSize; i++) {
                     RubyIO ioObj = saveReadIO(i, context);
                     saveReadBlocking(ioObj, i);
-                    saveBufferedRead(ioObj, i);                    
+                    saveBufferedRead(ioObj, i);
                     attachment.clear();
                     attachment.put('r', i);
                     trySelectRead(context, attachment, ioObj.getOpenFileChecked());
@@ -194,7 +195,7 @@ private void processWrites(Ruby runtime, IRubyObject[] args, ThreadContext conte
                 for (int i = 0; i < writeSize; i++) {
                     RubyIO ioObj = saveWriteIO(i, context);
                     saveWriteBlocking(ioObj, i);
-                    attachment.clear();                    
+                    attachment.clear();
                     attachment.put('w', i);
                     trySelectWrite(context, attachment, ioObj.getOpenFileChecked());
                 }
@@ -236,90 +237,103 @@ private void trySelectWrite(ThreadContext context, Map<Character,Integer> attach
         }
     }
 
-    private static long getTimeoutFromArg(IRubyObject timeArg, Ruby runtime) {
-        long timeout = 0;
-        if (timeArg instanceof RubyFloat) {
-            timeout = Math.round(((RubyFloat) timeArg).getDoubleValue() * 1000);
-        } else if (timeArg instanceof RubyFixnum) {
-            timeout = Math.round(((RubyFixnum) timeArg).getDoubleValue() * 1000);
-        } else {
-            // TODO: MRI also can hadle Bignum here
-            throw runtime.newTypeError("can't convert " + timeArg.getMetaClass().getName() + " into time interval");
+    private static long convertTimeout(final ThreadContext context, IRubyObject timeoutArg) {
+        final long timeout;
+        if (timeoutArg instanceof RubyFloat) {
+            timeout = Math.round(((RubyFloat) timeoutArg).getDoubleValue() * 1000);
         }
-        if (timeout < 0) {
-            throw runtime.newArgumentError("negative timeout given");
+        else if (timeoutArg instanceof RubyInteger) {
+            timeout = Math.round(((RubyInteger) timeoutArg).getDoubleValue() * 1000);
+        }
+        else {
+            final Ruby runtime = context.runtime;
+            if ( ! runtime.is1_8() ) {
+                RubyFloat t = null;
+                try {
+                    t = timeoutArg.callMethod(context, "to_f").convertToFloat();
+                }
+                catch (RaiseException e) { /* fallback to TypeError */ }
+
+                timeout = t != null ? Math.round(t.getDoubleValue() * 1000) : -1;
+            }
+            else timeout = -1;
+            if ( timeout == -1 ) {
+                throw runtime.newTypeError("can't convert " + timeoutArg.getMetaClass().getName() + " into time interval");
+            }
         }
+
+        if ( timeout < 0 ) throw context.runtime.newArgumentError("negative timeout given");
         return timeout;
 }
 
-private void doSelect(Ruby runtime, final boolean has_timeout, long timeout) throws IOException {
-    if (mainSelector != null) {
-        if (pendingReads == null && unselectableReads == null && unselectableWrites == null) {
-            if (has_timeout && timeout == 0) {
-                for (Selector selector : selectors.values()) selector.selectNow();
-            } else {
-                List<Future> futures = new ArrayList<Future>(enxioSelectors.size());
-                for (ENXIOSelector enxioSelector : enxioSelectors) {
-                    futures.add(runtime.getExecutor().submit(enxioSelector));
-                }
+    private void doSelect(Ruby runtime, final boolean has_timeout, long timeout) throws IOException {
+        if (mainSelector != null) {
+            if (pendingReads == null && unselectableReads == null && unselectableWrites == null) {
+                if (has_timeout && timeout == 0) {
+                    for (Selector selector : selectors.values()) selector.selectNow();
+                } else {
+                    List<Future> futures = new ArrayList<Future>(enxioSelectors.size());
+                    for (ENXIOSelector enxioSelector : enxioSelectors) {
+                        futures.add(runtime.getExecutor().submit(enxioSelector));
+                    }
 
-                mainSelector.select(has_timeout ? timeout : 0);
-                for (ENXIOSelector enxioSelector : enxioSelectors) enxioSelector.selector.wakeup();
-                // ensure all the enxio threads have finished
-                for (Future f : futures) try {
-                    f.get();
-                } catch (InterruptedException iex) {
-                } catch (ExecutionException eex) {
-                    if (eex.getCause() instanceof IOException) {
-                        throw (IOException) eex.getCause();
+                    mainSelector.select(has_timeout ? timeout : 0);
+                    for (ENXIOSelector enxioSelector : enxioSelectors) enxioSelector.selector.wakeup();
+                    // ensure all the enxio threads have finished
+                    for (Future f : futures) try {
+                        f.get();
+                    } catch (InterruptedException iex) {
+                    } catch (ExecutionException eex) {
+                        if (eex.getCause() instanceof IOException) {
+                            throw (IOException) eex.getCause();
+                        }
                     }
                 }
+            } else {
+                for (Selector selector : selectors.values()) selector.selectNow();
             }
-        } else {
-            for (Selector selector : selectors.values()) selector.selectNow();
         }
-    }
 
-    // If any enxio selectors woke up, remove them from the selected key set of the main selector
-    for (ENXIOSelector enxioSelector : enxioSelectors) {
-        Pipe.SourceChannel source = enxioSelector.pipe.source();
-        SelectionKey key = source.keyFor(mainSelector);
-        if (key != null && mainSelector.selectedKeys().contains(key)) {
-            mainSelector.selectedKeys().remove(key);
-            ByteBuffer buf = ByteBuffer.allocate(1);
-            source.read(buf);
+        // If any enxio selectors woke up, remove them from the selected key set of the main selector
+        for (ENXIOSelector enxioSelector : enxioSelectors) {
+            Pipe.SourceChannel source = enxioSelector.pipe.source();
+            SelectionKey key = source.keyFor(mainSelector);
+            if (key != null && mainSelector.selectedKeys().contains(key)) {
+                mainSelector.selectedKeys().remove(key);
+                ByteBuffer buf = ByteBuffer.allocate(1);
+                source.read(buf);
+            }
         }
     }
-}
-
+
     public static final int READ_ACCEPT_OPS = SelectExecutor.READ_ACCEPT_OPS;
     public static final int WRITE_CONNECT_OPS = SelectExecutor.WRITE_CONNECT_OPS;
     private static final int CANCELLED_OPS = SelectionKey.OP_READ | SelectionKey.OP_ACCEPT | SelectionKey.OP_CONNECT;
-    
+
     private static boolean ready(int ops, int mask) {
         return (ops & mask) != 0;
     }
-    
+
     private static boolean readAcceptReady(int ops) {
         return ready(ops, READ_ACCEPT_OPS);
     }
-    
+
     private static boolean writeConnectReady(int ops) {
         return ready(ops, WRITE_CONNECT_OPS);
     }
-    
+
     private static boolean cancelReady(int ops) {
         return ready(ops, CANCELLED_OPS);
     }
-    
+
     private static boolean writeReady(int ops) {
         return ready(ops, SelectionKey.OP_WRITE);
     }
 
     @SuppressWarnings("unchecked")
     private void processSelectedKeys(Ruby runtime) throws IOException {
         for (Selector selector : selectors.values()) {
-            
+
             for (SelectionKey key : selector.selectedKeys()) {
                 int readIoIndex = 0;
                 int writeIoIndex = 0;
@@ -551,7 +565,7 @@ public Object call() throws Exception {
             return null;
         }
     }
-    
+
     Ruby runtime;
     RubyArray readArray = null;
     int readSize = 0;

diff --git a/test/jruby/test_io.rb b/test/jruby/test_io.rb
@@ -3,8 +3,6 @@
 require 'test/jruby/test_helper'
 require 'rbconfig'
 require 'stringio'
-require 'java'
-require 'jruby'
 
 class TestIO < Test::Unit::TestCase
   include TestHelper
@@ -230,11 +228,20 @@ def test_delete
   end
 
   def test_select
-    ##### select #####
     assert_equal(nil, select(nil, nil, nil, 0))
     assert_raises(ArgumentError) { select(nil, nil, nil, -1) }
   end
 
+  class NumLike
+    def initialize; @num = 1 end
+    def method_missing(m, *args); @num.send(m, *args) end
+  end
+
+  def test_select_converts_timeout
+    assert_equal nil, IO.select([], [], [], NumLike.new)
+    assert_raises(TypeError) { IO.select([], [], [], Object.new) }
+  end
+
   class FakeStream
     attr_accessor :data
     def initialize(stream, passthrough = false)
@@ -425,27 +432,27 @@ def ensure_files(*files)
     files.each {|f| File.open(f, "w") {|g| g << " " } }
   end
   private :ensure_files
-  
+
   # JRUBY-5114
-  def test_autoclose_false_leaves_channels_open
+  def test_autoclose_false_leaves_channels_open; require 'java'
     channel = java.io.FileInputStream.new(__FILE__).channel
-    
+
     # sanity check
     io1 = channel.to_io(:autoclose => false)
     assert_equal "#", io1.sysread(1)
     io2 = channel.to_io(:autoclose => false)
     assert_equal " ", io2.sysread(1)
-    
+
     # dereference and force GC a few times to finalize
     io1 = nil
     5.times { java.lang.System.gc }
-    
+
     # io2 and original channel should still be open and usable
     assert_equal "-", io2.sysread(1)
     assert !io2.closed?
-    
+
     assert channel.open?
-  end
+  end if defined? JRUBY_VERSION
 
   def test_gets_no_args
     File.open(@file, 'w') { |f| f.write 'abcde' }
@@ -473,15 +480,15 @@ def test_stringio_gets_separator
 
   if false # FIXME: Disabled until (if ever) we need it for new IO logic (in 9k)
   # JRUBY-6137
-  def test_rubyio_fileno_mapping_leak
+  def test_rubyio_fileno_mapping_leak; require 'jruby'
     starting_count = JRuby.runtime.fileno_int_map_size
     io = org.jruby.RubyIO.new(JRuby.runtime, org.jruby.util.io.STDIO::ERR)
     open_io_count = JRuby.runtime.fileno_int_map_size
     assert_equal(starting_count + 1, open_io_count)
     io.close
     closed_io_count = JRuby.runtime.fileno_int_map_size
     assert_equal(starting_count, closed_io_count)
-  end
+  end if defined? JRUBY_VERSION
   end
 
   # JRUBY-1222