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 the exception: false API of read/write nonblock #926

Merged
merged 1 commit into from Sep 27, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .rubocop.yml
Expand Up @@ -57,6 +57,9 @@ Style/ParallelAssignment:
Style/NumericPredicate:
Enabled: false

Style/IfUnlessModifier:
Enabled: false

Style/SignalException:
Exclude:
- 'lib/redis/connection/synchrony.rb'
Expand Down
64 changes: 34 additions & 30 deletions lib/redis/connection/ruby.rb
Expand Up @@ -49,43 +49,47 @@ def gets
end

def _read_from_socket(nbytes)
begin
read_nonblock(nbytes)
rescue IO::WaitReadable
if IO.select([self], nil, nil, @timeout)
retry
else
raise Redis::TimeoutError
end
rescue IO::WaitWritable
if IO.select(nil, [self], nil, @timeout)
retry
else
raise Redis::TimeoutError
loop do
case chunk = read_nonblock(nbytes, exception: false)
when :wait_readable
unless IO.select([self], nil, nil, @timeout)
raise Redis::TimeoutError
end
when :wait_writable
unless IO.select(nil, [self], nil, @timeout)
raise Redis::TimeoutError
end
when nil
raise Errno::ECONNRESET
when String
return chunk
end
end
rescue EOFError
raise Errno::ECONNRESET
end

def _write_to_socket(data)
begin
write_nonblock(data)
rescue IO::WaitWritable
if IO.select(nil, [self], nil, @write_timeout)
retry
else
raise Redis::TimeoutError
end
rescue IO::WaitReadable
if IO.select([self], nil, nil, @write_timeout)
retry
else
raise Redis::TimeoutError
total_bytes_written = 0
loop do
case bytes_written = write_nonblock(data, exception: false)
when :wait_readable
unless IO.select([self], nil, nil, @timeout)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this still use @write_timeout (and not @timeout)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Indeed. Good catch.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fixed in 087a11b

raise Redis::TimeoutError
end
when :wait_writable
unless IO.select(nil, [self], nil, @timeout)
raise Redis::TimeoutError
end
when nil
raise Errno::ECONNRESET
when Integer
total_bytes_written += bytes_written
if bytes_written < data.bytesize
data.slice!(0, bytes_written)
else
return total_bytes_written
end
end
end
rescue EOFError
raise Errno::ECONNRESET
end

def write(data)
Expand Down
2 changes: 1 addition & 1 deletion test/internals_test.rb
Expand Up @@ -244,7 +244,7 @@ def close_on_connection(seq)
else
raise "Expected SELECT"
end
unless seq.include?(n) # rubocop:disable Style/IfUnlessModifier
unless seq.include?(n)
session.write("+#{n}\r\n") while read_command.call(session)
end
end
Expand Down