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.
rubinius · Oct 2, 2015 · 2b90e73 · 2b90e73
1 parent acd40bd
commit 2b90e73
Showing 1 changed file with 30 additions and 55 deletions.
diff --git a/kernel/common/io.rb b/kernel/common/io.rb
@@ -713,12 +713,12 @@ def self.timer_sub(time1, time2, result)
     end
 
     def self.make_timeval_timeout(timeout)
-      limit = Timeval_t.new
-      future = Timeval_t.new
-
       if timeout
+        limit = Timeval_t.new
+        future = Timeval_t.new
+
         limit[:tv_sec] = (timeout / 1_000_000.0).to_i
-        limit[:tv_usec] = (timeout % 1_000_000.0)
+        limit[:tv_usec] = (timeout % 1_000_000.0).to_i
 
         # Get current time to be used if select is interrupted and we have to recalculate the sleep time
         if FFI.call_failed?(FFI::Platform::POSIX.gettimeofday(future, nil))
@@ -745,21 +745,14 @@ def self.select(readables, writables, errorables, timeout)
       read_set, highest_read_fd = readables.nil? ? [nil, nil] : fd_set_from_array(readables)
       write_set, highest_write_fd = writables.nil? ? [nil, nil] : fd_set_from_array(writables)
       error_set, highest_err_fd = errorables.nil? ? [nil, nil] : fd_set_from_array(errorables)
-      max_fd = [highest_read_fd, highest_write_fd, highest_err_fd].compact.max
+      max_fd = [highest_read_fd, highest_write_fd, highest_err_fd].compact.max || -1
 
       unless const_defined?(:Timeval_t)
         # This is a complete hack.
         IO.class_eval(Rubinius::Config['rbx.platform.timeval.class'])
       end
 
       time_limit, future = make_timeval_timeout(timeout)
-      # debugging only...
-#      file = File.open("/tmp/select", 'w+')
-#        set = read_set.to_set
-#        file.puts set.class
-#        file.puts set.inspect
-#        file.puts(sprintf("0x%x", set.address))
-#        set.read_array_of_char(10).each { |char| file.puts(sprintf("%b ", char)) }
 
       events = 0
       loop do
@@ -780,7 +773,7 @@ def self.select(readables, writables, errorables, timeout)
 
         break
       end
-      
+
       return nil if events.zero?
 
       output_fds = []
@@ -789,6 +782,27 @@ def self.select(readables, writables, errorables, timeout)
       output_fds << collect_set_fds(errorables, error_set)
       return output_fds
     end
+
+    def self.validate_and_convert_argument(objects)
+      if objects
+        raise TypeError, "Argument must be an Array" unless objects.respond_to?(:to_ary)
+        objects =
+          objects.to_ary.map do |obj|
+          if obj.kind_of? IO
+            raise IOError, "closed stream" if obj.closed?
+            obj
+          else
+            raise TypeError unless obj.respond_to?(:to_io)
+            io = obj.to_io
+            raise TypeError unless io
+            raise IOError, "closed stream" if io.closed?
+            [obj, io]
+          end
+        end
+      end
+
+      objects
+    end
   end # class Select
 
   attr_accessor :external
@@ -1415,48 +1429,9 @@ def self.select(readables=nil, writables=nil, errorables=nil, timeout=nil)
       timeout = Integer(timeout * 1_000_000)
     end
 
-    if readables
-      readables =
-      readables.to_ary.map do |obj|
-        if obj.kind_of? IO
-          raise IOError, "closed stream" if obj.closed?
-          return [[obj],[],[]] unless obj.buffer_empty? # FIXME: eliminated buffer_empty? so what do we check here?
-          obj
-        else
-          io = obj.to_io
-          raise IOError, "closed stream" if io.closed?
-          [obj, io]
-        end
-      end
-    end
-
-    if writables
-      writables =
-      writables.to_ary.map do |obj|
-        if obj.kind_of? IO
-          raise IOError, "closed stream" if obj.closed?
-          obj
-        else
-          io = obj.to_io
-          raise IOError, "closed stream" if io.closed?
-          [obj, io]
-        end
-      end
-    end
-
-    if errorables
-      errorables =
-      errorables.to_ary.map do |obj|
-        if obj.kind_of? IO
-          raise IOError, "closed stream" if obj.closed?
-          obj
-        else
-          io = obj.to_io
-          raise IOError, "closed stream" if io.closed?
-          [obj, io]
-        end
-      end
-    end
+    readables = IO::Select.validate_and_convert_argument(readables)
+    writables = IO::Select.validate_and_convert_argument(writables)
+    errorables = IO::Select.validate_and_convert_argument(errorables)
 
     IO::Select.select(readables, writables, errorables, timeout)
   end