Merge pull request #502 from redis/fix-timeout-inconsistency

Fix inconsistent connections when using Timeout under MRI 2.1+.

Author: djanowski

lib/redis/client.rb

@@ -77,6 +77,8 @@ def initialize(options = {})
       @connection = nil
       @command_map = {}
 
+      @pending_reads = 0
+
       if options.include?(:sentinels)
         @connector = Connector::Sentinel.new(@options)
       else
@@ -243,12 +245,15 @@ def io
 
     def read
       io do
-        connection.read
+        value = connection.read
+        @pending_reads -= 1
+        value
       end
     end
 
     def write(command)
       io do
+        @pending_reads += 1
         connection.write(command)
       end
     end
@@ -315,6 +320,7 @@ def establish_connection
       @options[:port] = server[:port]
 
       @connection = @options[:driver].connect(server)
+      @pending_reads = 0
     rescue TimeoutError,
            Errno::ECONNREFUSED,
            Errno::EHOSTDOWN,
@@ -326,6 +332,8 @@ def establish_connection
     end
 
     def ensure_connected
+      disconnect if @pending_reads > 0
+
       attempts = 0
 
       begin

test/connection_handling_test.rb

@@ -186,4 +186,25 @@ def test_config_set
       r.config :set, "timeout", 300
     end
   end
+
+  driver(:ruby, :hiredis) do
+    def test_consistency_on_multithreaded_env
+      t = nil
+
+      commands = {
+        :set => lambda { |key, value| t.kill; "+OK\r\n" },
+        :incr => lambda { |key| ":1\r\n" },
+      }
+
+      redis_mock(commands) do |redis|
+        t = Thread.new do
+          redis.set("foo", "bar")
+        end
+
+        t.join
+
+        assert_equal 1, redis.incr("baz")
+      end
+    end
+  end
 end

test/support/redis_mock.rb

@@ -24,20 +24,23 @@ def shutdown
     end
 
     def run
-      loop do
-        session = @server.accept
-
-        begin
-          return if yield(session) == :exit
-        ensure
-          session.close
+      begin
+        loop do
+          session = @server.accept
+
+          begin
+            return if yield(session) == :exit
+          ensure
+            session.close
+          end
         end
+      rescue => ex
+        $stderr.puts "Error running mock server: #{ex.message}" if VERBOSE
+        $stderr.puts ex.backtrace if VERBOSE
+        retry
+      ensure
+        @server.close
       end
-    rescue => ex
-      $stderr.puts "Error running mock server: #{ex.message}" if VERBOSE
-      $stderr.puts ex.backtrace if VERBOSE
-    ensure
-      @server.close
     end
   end