Merge remote-tracking branch 'redis-rb/master' into reconnect_attempts_to_fixnum

Author: otoyo

.travis.yml

@@ -38,27 +38,48 @@ matrix:
     # hiredis
     - rvm: jruby-18mode
       gemfile: .travis/Gemfile
-      env: conn=hiredis REDIS_BRANCH=2.8
+      env: conn=hiredis REDIS_BRANCH=3.0
+    - rvm: jruby-18mode
+      gemfile: .travis/Gemfile
+      env: conn=hiredis REDIS_BRANCH=3.2
+    - rvm: jruby-19mode
+      gemfile: .travis/Gemfile
+      env: conn=hiredis REDIS_BRANCH=3.0
     - rvm: jruby-19mode
       gemfile: .travis/Gemfile
-      env: conn=hiredis REDIS_BRANCH=2.8
+      env: conn=hiredis REDIS_BRANCH=3.2
     - rvm: jruby-9.0.5.0
       gemfile: .travis/Gemfile
-      env: conn=hiredis REDIS_BRANCH=2.8
+      env: conn=hiredis REDIS_BRANCH=3.0
+    - rvm: jruby-9.0.5.0
+      gemfile: .travis/Gemfile
+      env: conn=hiredis REDIS_BRANCH=3.2
 
     # synchrony
     - rvm: 1.8.7
       gemfile: .travis/Gemfile
-      env: conn=synchrony REDIS_BRANCH=2.8
+      env: conn=synchrony REDIS_BRANCH=3.0
+    - rvm: 1.8.7
+      gemfile: .travis/Gemfile
+      env: conn=synchrony REDIS_BRANCH=3.2
+    - rvm: jruby-18mode
+      gemfile: .travis/Gemfile
+      env: conn=synchrony REDIS_BRANCH=3.0
     - rvm: jruby-18mode
       gemfile: .travis/Gemfile
-      env: conn=synchrony REDIS_BRANCH=2.8
+      env: conn=synchrony REDIS_BRANCH=3.2
     - rvm: jruby-19mode
       gemfile: .travis/Gemfile
-      env: conn=synchrony REDIS_BRANCH=2.8
+      env: conn=synchrony REDIS_BRANCH=3.0
+    - rvm: jruby-19mode
+      gemfile: .travis/Gemfile
+      env: conn=synchrony REDIS_BRANCH=3.2
+    - rvm: jruby-9.0.5.0
+      gemfile: .travis/Gemfile
+      env: conn=synchrony REDIS_BRANCH=3.0
     - rvm: jruby-9.0.5.0
       gemfile: .travis/Gemfile
-      env: conn=synchrony REDIS_BRANCH=2.8
+      env: conn=synchrony REDIS_BRANCH=3.2
   allow_failures:
     - rvm: rbx-2
 

CHANGELOG.md

@@ -12,6 +12,21 @@
   security updates in June of 2013; continuing to support it would prevent
   the use of newer features of Ruby.
 
+# 3.3.3
+
+* Improved timeout handling after dropping Timeout module.
+
+# 3.3.2
+
+* Added support for SPOP with COUNT. See #628.
+
+* Fixed connection glitches when using SSL. See #644.
+
+# 3.3.1
+
+* Remove usage of Timeout::timeout, refactor into using low level non-blocking writes.
+  This fixes a memory leak due to Timeout creating threads on each invocation.
+
 # 3.3.0
 
 * Added support for SSL/TLS. Redis doesn't support SSL natively, so you still

lib/redis.rb

@@ -1336,13 +1336,18 @@ def srem(key, member)
     end
   end
 
-  # Remove and return a random member from a set.
+  # Remove and return one or more random member from a set.
   #
   # @param [String] key
   # @return [String]
-  def spop(key)
+  # @param [Fixnum] count
+  def spop(key, count = nil)
     synchronize do |client|
-      client.call([:spop, key])
+      if count.nil?
+        client.call([:spop, key])
+      else
+        client.call([:spop, key, count])
+      end
     end
   end
 

lib/redis/client.rb

@@ -453,12 +453,12 @@ def _parse_options(options)
       case options[:tcp_keepalive]
       when Hash
         [:time, :intvl, :probes].each do |key|
-          unless options[:tcp_keepalive][key].is_a?(Fixnum)
-            raise "Expected the #{key.inspect} key in :tcp_keepalive to be a Fixnum"
+          unless options[:tcp_keepalive][key].is_a?(Integer)
+            raise "Expected the #{key.inspect} key in :tcp_keepalive to be an Integer"
           end
         end
 
-      when Fixnum
+      when Integer
         if options[:tcp_keepalive] >= 60
           options[:tcp_keepalive] = {:time => options[:tcp_keepalive] - 20, :intvl => 10, :probes => 2}
 

lib/redis/connection/ruby.rb

@@ -10,15 +10,30 @@
   # Not all systems have OpenSSL support
 end
 
+if RUBY_VERSION < "1.9.3"
+  class String
+    # Ruby 1.8.7 does not have byteslice, but it handles encodings differently anyway.
+    # We can simply slice the string, which is a byte array there.
+    def byteslice(*args)
+      slice(*args)
+    end
+  end
+end
+
 class Redis
   module Connection
     module SocketMixin
 
       CRLF = "\r\n".freeze
 
       # Exceptions raised during non-blocking I/O ops that require retrying the op
-      NBIO_EXCEPTIONS = [Errno::EWOULDBLOCK, Errno::EAGAIN]
-      NBIO_EXCEPTIONS << IO::WaitReadable if RUBY_VERSION >= "1.9.3"
+      if RUBY_VERSION >= "1.9.3"
+        NBIO_READ_EXCEPTIONS = [IO::WaitReadable]
+        NBIO_WRITE_EXCEPTIONS = [IO::WaitWritable]
+      else
+        NBIO_READ_EXCEPTIONS = [Errno::EWOULDBLOCK, Errno::EAGAIN]
+        NBIO_WRITE_EXCEPTIONS = [Errno::EWOULDBLOCK, Errno::EAGAIN]
+      end
 
       def initialize(*args)
         super(*args)
@@ -68,21 +83,58 @@ def _read_from_socket(nbytes)
         begin
           read_nonblock(nbytes)
 
-        rescue *NBIO_EXCEPTIONS
+        rescue *NBIO_READ_EXCEPTIONS
           if IO.select([self], nil, nil, @timeout)
             retry
           else
             raise Redis::TimeoutError
           end
+        rescue *NBIO_WRITE_EXCEPTIONS
+          if IO.select(nil, [self], nil, @timeout)
+            retry
+          else
+            raise Redis::TimeoutError
+          end
         end
 
       rescue EOFError
         raise Errno::ECONNRESET
       end
 
-      # UNIXSocket and TCPSocket don't support write timeouts
-      def write(*args)
-        Timeout.timeout(@write_timeout, TimeoutError) { super }
+      def _write_to_socket(data)
+        begin
+          write_nonblock(data)
+
+        rescue *NBIO_WRITE_EXCEPTIONS
+          if IO.select(nil, [self], nil, @write_timeout)
+            retry
+          else
+            raise Redis::TimeoutError
+          end
+        rescue *NBIO_READ_EXCEPTIONS
+          if IO.select([self], nil, nil, @write_timeout)
+            retry
+          else
+            raise Redis::TimeoutError
+          end
+        end
+
+      rescue EOFError
+        raise Errno::ECONNRESET
+      end
+
+      def write(data)
+        return super(data) unless @write_timeout
+
+        length = data.bytesize
+        total_count = 0
+        loop do
+          count = _write_to_socket(data)
+
+          total_count += count
+          return total_count if total_count >= length
+          data = data.byteslice(count..-1)
+        end
       end
     end
 
@@ -255,6 +307,7 @@ def self.connect(config)
           raise ArgumentError, "SSL incompatible with unix sockets" if config[:ssl]
           sock = UNIXSocket.connect(config[:path], config[:connect_timeout])
         elsif config[:scheme] == "rediss" || config[:ssl]
+          raise ArgumentError, "This library does not support SSL on Ruby < 1.9" if RUBY_VERSION < "1.9.3"
           sock = SSLSocket.connect(config[:host], config[:port], config[:connect_timeout], config[:ssl_params])
         else
           sock = TCPSocket.connect(config[:host], config[:port], config[:connect_timeout])

lib/redis/distributed.rb

@@ -483,8 +483,8 @@ def srem(key, member)
     end
 
     # Remove and return a random member from a set.
-    def spop(key)
-      node_for(key).spop(key)
+    def spop(key, count = nil)
+      node_for(key).spop(key, count)
     end
 
     # Get a random member from a set.

lib/redis/version.rb

@@ -1,3 +1,3 @@
 class Redis
-  VERSION = "3.3.0"
+  VERSION = "3.3.3"
 end

test/internals_test.rb

@@ -5,6 +5,7 @@
 class TestInternals < Test::Unit::TestCase
 
   include Helper::Client
+  include Helper::Skipable
 
   def test_logger
     r.ping
@@ -160,6 +161,25 @@ def test_connection_timeout
     assert (Time.now - start_time) <= opts[:timeout]
   end
 
+  driver(:ruby) do
+    def test_write_timeout
+      return skip("Relies on buffer sizes, might be unreliable")
+
+      server = TCPServer.new("127.0.0.1", 0)
+      port   = server.addr[1]
+
+      # Hacky, but we need the buffer size
+      val = TCPSocket.new("127.0.0.1", port).getsockopt(Socket::SOL_SOCKET, Socket::SO_SNDBUF).unpack("i")[0]
+
+      assert_raise(Redis::TimeoutError) do
+        Timeout.timeout(1) do
+          redis = Redis.new(:port => port, :timeout => 5, :write_timeout => 0.1)
+          redis.set("foo", "1" * val*2)
+        end
+      end
+    end
+  end
+
   def close_on_ping(seq, options = {})
     $request = 0
 

test/lint/sets.rb

@@ -52,6 +52,21 @@ def test_spop
       assert_equal nil, r.spop("foo")
     end
 
+    def test_spop_with_positive_count
+      target_version "3.2.0" do
+        r.sadd "foo", "s1"
+        r.sadd "foo", "s2"
+        r.sadd "foo", "s3"
+        r.sadd "foo", "s4"
+
+        pops = r.spop("foo", 3)
+
+        assert !(["s1", "s2", "s3", "s4"] & pops).empty?
+        assert_equal 3, pops.size
+        assert_equal 1, r.scard("foo")
+      end
+    end
+
     def test_scard
       assert_equal 0, r.scard("foo")
 

test/remote_server_control_commands_test.rb

@@ -101,7 +101,7 @@ def test_object
     assert_equal 1, r.object(:refcount, "list")
     encoding = r.object(:encoding, "list")
     assert "ziplist" == encoding || "quicklist" == encoding, "Wrong encoding for list"
-    assert r.object(:idletime, "list").kind_of?(Fixnum)
+    assert r.object(:idletime, "list").kind_of?(Integer)
   end
 
   def test_sync