Re-add io/wait operations

[HoneyryderChuck]

@tarcieri

Coming from #298 and #322

Why

Just to understand my motivation, all ruby net/protocol libraries use io/wait, so I'd say it's a must for libraries like http to update support.

How

I was testing with the example from #298 until I got the semantics right. Here is the script I used:

# test_timeout.rb
require 'http'
$strategy = ENV["STRATEGY"] || "master"

# from master
class HTTP::Timeout::PerOperation

  def readpartial(size)
    loop do
      result = @socket.read_nonblock(size, :exception => false)

      return :eof   if result.nil?
      return result if result != :wait_readable

      case $strategy
        when 'master'
          unless IO.select([@socket], nil, nil, read_timeout)
            raise TimeoutError, "Read timed out after #{read_timeout} seconds"
          end
        when 'bug'
          unless @socket.to_io.wait_readable(read_timeout)
            raise TimeoutError, "Read timed out after #{read_timeout} seconds"
          end
        when 'fix'
          case @socket.to_io.wait_readable(read_timeout)
            # http://ruby-doc.org/stdlib-2.2.5/libdoc/io/wait/rdoc/IO.html#method-i-wait
            # quote: .... returns self or nil when EOF is reached.
            when nil
              return :eof
            when false
              raise TimeoutError, "Read timed out after #{read_timeout} seconds"
            # else continue, you have socket
          end
      end
    end
  end
end

url = "https://play.google.com/store/apps/" \
      "category/GAMES/collection/top?start=400&num=100&hl=en&gl=us"

42.times do |i|
  start = Time.now.to_f

  print format("%3d ... ", i + 1)
  begin
    # http gem
    res = HTTP.timeout(:read => 5, :connect => 5, :write => 5).
               get(url)
    res.flush
     puts "pass (#{Time.now.to_f - start}) -> code: #{res.code}"
   rescue => e
     puts "fail (#{Time.now.to_f - start})"
     raise e
   end
 end

Run this script with the 3 strategies and compare the results.

This might be controversial (I also don't fully understand it, as most of the times #wait_readable returns nil when it should return false), but you can with the implementation on net/protocol.

My faulty presumption: when the connection is Keep-Alive, the server doesn't send EOF/close the socket). I say this because if you see the "bug" strategy, the body is fully there every single time.

[tarcieri]

Interesting. If this actually works, I'm all for it.

Want to send a PR?

[HoneyryderChuck]

I'm be "out-of-office" until middle of August, I'll only be able to tackle this then. I'll also need some more data to back up this assumption, but I really think that, for cases like Keep-Alive, we have to wait until #read_nonblock stops sending data.

[abotalov]

It looks like behaviour of wait_readable was changed a bit in 2.3.0:
ruby/ruby@a60e00f

Waits until IO is readable without blocking and returns self, or **nil** when times out. Returns true immediately when buffered data is available.

Btw, it looks like #wait_writable can also return nil - ruby/ruby@a4f7274

[tarcieri]

This will get handled in #272. If you'd like to make sure that's the case, please take a look at https://github.com/socketry/socketry

[HoneyryderChuck]

@tarcieri sweet! I was figuring that the changes would end up there. Still, a few questions:

• Regarding https://github.com/socketry/socketry/blob/07ba77f583dd5bfaf6a029cc43b9b77125fdc84e/lib/socketry/tcp/socket.rb#L171-L175, my MR changes and comments showed that between a :wait_readable return and a socket.wait_readable(timeout) call, the socket might not have anything to read, will return nil to the socket.wait_readable(timeout) (which in this link is still being handled as a timeout error) and will return :eof on the subsequent call to read_nonblock. I don't see this being handled in socketry. Dont you want to handle this use case? (Again, link to where I handled this
• connect_nonblockand accept_nonblock are not ignoring the exception in ruby 2.3 mode. Will this be added before release?

[tarcieri]

@TiagoCardoso1983 the first case appears to be unhandled. Re: exceptionless I/O for 2.3 mode, I don't really see the value in complicating the logic just for connect/accept