Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use io/wait on platforms where it's available #283

Merged
merged 1 commit into from
Dec 26, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
AllCops:
DisplayCopNames: true

Metrics/BlockNesting:
Max: 2

Expand Down
34 changes: 25 additions & 9 deletions lib/http/timeout/global.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,32 @@ def perform_io
:eof
end

# Wait for a socket to become readable
def wait_readable_or_timeout
IO.select([@socket], nil, nil, time_left)
log_time
end
if RUBY_VERSION < "2.0.0"
# Wait for a socket to become readable
def wait_readable_or_timeout
IO.select([@socket], nil, nil, time_left)
log_time
end

# Wait for a socket to become writable
def wait_writable_or_timeout
IO.select(nil, [@socket], nil, time_left)
log_time
# Wait for a socket to become writable
def wait_writable_or_timeout
IO.select(nil, [@socket], nil, time_left)
log_time
end
else
require "io/wait"

# Wait for a socket to become readable
def wait_readable_or_timeout
@socket.to_io.wait_readable(time_left)
log_time
end

# Wait for a socket to become writable
def wait_writable_or_timeout
@socket.to_io.wait_writable(time_left)
log_time
end
end

# Due to the run/retry nature of nonblocking I/O, it's easier to keep track of time
Expand Down
47 changes: 32 additions & 15 deletions lib/http/timeout/null.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,26 +50,43 @@ def write(data)
end
alias_method :<<, :write

# These cops can be re-eanbled after we go Ruby 2.0+ only
# rubocop:disable Lint/UselessAccessModifier, Metrics/BlockNesting

private

# Retry reading
def rescue_readable
yield
rescue IO::WaitReadable
if IO.select([socket], nil, nil, read_timeout)
retry
else
if RUBY_VERSION < "2.0.0"
# Retry reading
def rescue_readable
yield
rescue IO::WaitReadable
retry if IO.select([socket], nil, nil, read_timeout)
raise TimeoutError, "Read timed out after #{read_timeout} seconds"
end

# Retry writing
def rescue_writable
yield
rescue IO::WaitWritable
retry if IO.select(nil, [socket], nil, write_timeout)
raise TimeoutError, "Write timed out after #{write_timeout} seconds"
end
else
require "io/wait"

# Retry reading
def rescue_readable
yield
rescue IO::WaitReadable
retry if socket.to_io.wait_readable(read_timeout)
raise TimeoutError, "Read timed out after #{read_timeout} seconds"
end
end

# Retry writing
def rescue_writable
yield
rescue IO::WaitWritable
if IO.select(nil, [socket], nil, write_timeout)
retry
else
# Retry writing
def rescue_writable
yield
rescue IO::WaitWritable
retry if socket.to_io.wait_writable(write_timeout)
raise TimeoutError, "Write timed out after #{write_timeout} seconds"
end
end
Expand Down
14 changes: 10 additions & 4 deletions lib/http/timeout/per_operation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,17 @@ def write(data)
# Read data from the socket
def readpartial(size)
loop do
result = socket.read_nonblock(size, :exception => false)
result = rescue_readable do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't make sense, there's nothing to rescue since it's not using exceptions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you recall from before, JRuby silently discards :exception => false on SSL sockets and still throws exceptions. I can add a comment to that effect.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some clarifying comments in fbb57b5

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh right fsss. Yea if you can put a comment please.

Sent from my iPhone

On Dec 26, 2015, at 15:49, Tony Arcieri notifications@github.com wrote:

In lib/http/timeout/per_operation.rb:

@@ -59,14 +59,17 @@ def write(data)
# Read data from the socket
def readpartial(size)
loop do

  •        result = socket.read_nonblock(size, :exception => false)
    
  •        result = rescue_readable do
    
    If you recall from before, JRuby silently discards :exception => false on SSL sockets and still throws exceptions. I can add a comment to that effect.


Reply to this email directly or view it on GitHub.

socket.read_nonblock(size, :exception => false)
end

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

unless IO.select([socket], nil, nil, read_timeout)
unless socket.to_io.wait_readable(read_timeout)
fail TimeoutError, "Read timed out after #{read_timeout} seconds"
end
end
Expand All @@ -75,10 +78,13 @@ def readpartial(size)
# Write data to the socket
def write(data)
loop do
result = socket.write_nonblock(data, :exception => false)
result = rescue_writable do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here on rescue

socket.write_nonblock(data, :exception => false)
end

return result unless result == :wait_writable

unless IO.select(nil, [socket], nil, write_timeout)
unless socket.to_io.wait_writable(write_timeout)
fail TimeoutError, "Read timed out after #{write_timeout} seconds"
end
end
Expand Down