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

Prefer IO#wait_readable/IO#wait_writable rather than IO.select. #960

Merged
merged 3 commits into from Nov 17, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 21 additions & 9 deletions lib/redis/connection/ruby.rb
Expand Up @@ -52,11 +52,11 @@ def _read_from_socket(nbytes)
loop do
case chunk = read_nonblock(nbytes, exception: false)
when :wait_readable
unless IO.select([self], nil, nil, @timeout)
unless wait_readable(@timeout)
raise Redis::TimeoutError
end
when :wait_writable
unless IO.select(nil, [self], nil, @timeout)
unless wait_writable(@timeout)
raise Redis::TimeoutError
end
when nil
Expand All @@ -72,11 +72,11 @@ def _write_to_socket(data)
loop do
case bytes_written = write_nonblock(data, exception: false)
when :wait_readable
unless IO.select([self], nil, nil, @write_timeout)
unless wait_readable(@write_timeout)
raise Redis::TimeoutError
end
when :wait_writable
unless IO.select(nil, [self], nil, @write_timeout)
unless wait_writable(@write_timeout)
raise Redis::TimeoutError
end
when nil
Expand Down Expand Up @@ -139,7 +139,7 @@ def self.connect(path, timeout)
raise TimeoutError
end

# JRuby raises Errno::EAGAIN on #read_nonblock even when IO.select
# JRuby raises Errno::EAGAIN on #read_nonblock even when it
# says it is readable (1.6.6, in both 1.8 and 1.9 mode).
# Use the blocking #readpartial method instead.

Expand All @@ -164,7 +164,7 @@ def self.connect_addrinfo(addrinfo, port, timeout)
begin
sock.connect_nonblock(sockaddr)
rescue Errno::EINPROGRESS
raise TimeoutError if IO.select(nil, [sock], nil, timeout).nil?
raise TimeoutError unless sock.wait_writable(timeout)

begin
sock.connect_nonblock(sockaddr)
Expand Down Expand Up @@ -219,7 +219,7 @@ def self.connect(path, timeout)
begin
sock.connect_nonblock(sockaddr)
rescue Errno::EINPROGRESS
raise TimeoutError if IO.select(nil, [sock], nil, timeout).nil?
raise TimeoutError unless sock.wait_writable(timeout)

begin
sock.connect_nonblock(sockaddr)
Expand All @@ -237,6 +237,18 @@ def self.connect(path, timeout)
class SSLSocket < ::OpenSSL::SSL::SSLSocket
include SocketMixin

unless method_defined?(:wait_readable)
def wait_readable(timeout = nil)
to_io.wait_readable(timeout)
end
end

unless method_defined?(:wait_writable)
def wait_writable(timeout = nil)
to_io.wait_writable(timeout)
end
end

def self.connect(host, port, timeout, ssl_params)
# Note: this is using Redis::Connection::TCPSocket
tcp_sock = TCPSocket.connect(host, port, timeout)
Expand All @@ -258,13 +270,13 @@ def self.connect(host, port, timeout, ssl_params)
# Instead, you have to retry.
ssl_sock.connect_nonblock
rescue Errno::EAGAIN, Errno::EWOULDBLOCK, IO::WaitReadable
if IO.select([ssl_sock], nil, nil, timeout)
if ssl_sock.wait_readable(timeout)
retry
else
raise TimeoutError
end
rescue IO::WaitWritable
if IO.select(nil, [ssl_sock], nil, timeout)
if ssl_sock.wait_writable(timeout)
retry
else
raise TimeoutError
Expand Down