THRIFT-899. rb: Ruby read timeouts can sometimes be 2x what they should be

This patch makes sure that we don't wait longer than necessary for timeouts.

Patch: Ryan King

git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@998303 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/lib/rb/lib/thrift/transport/socket.rb b/lib/rb/lib/thrift/transport/socket.rb
index 06c937e..9bb2036 100644 (file)
--- a/lib/rb/lib/thrift/transport/socket.rb
+++ b/lib/rb/lib/thrift/transport/socket.rb
@@ -97,12 +97,13 @@ module Thrift
           data = @handle.readpartial(sz)
         else
           # it's possible to interrupt select for something other than the timeout
-          # so we need to ensure we've waited long enough
+          # so we need to ensure we've waited long enough, but not too long
           start = Time.now
-          rd = nil # scoping
-          loop do
-            rd, = IO.select([@handle], nil, nil, @timeout)
-            break if (rd and not rd.empty?) or Time.now - start >= @timeout
+          timespent = 0
+          rd = loop do
+            rd, = IO.select([@handle], nil, nil, @timeout - timespent)
+            timespent = Time.now - start
+            break rd if (rd and not rd.empty?) or timespent >= @timeout
           end
           if rd.nil? or rd.empty?
             raise TransportException.new(TransportException::TIMED_OUT, "Socket: Timed out reading #{sz} bytes from #{@desc}")

diff --git a/lib/rb/spec/socket_spec_shared.rb b/lib/rb/spec/socket_spec_shared.rb
index 96b433b..6ed77fa 100644 (file)
--- a/lib/rb/spec/socket_spec_shared.rb
+++ b/lib/rb/spec/socket_spec_shared.rb
@@ -91,7 +91,7 @@ shared_examples_for "a socket" do
   it "should raise an error when read times out" do
     @socket.timeout = 0.5
     @socket.open
-    IO.should_receive(:select).with([@handle], nil, nil, 0.5).at_least(1).times.and_return(nil)
+    IO.should_receive(:select).once {sleep(0.5); nil}
     lambda { @socket.read(17) }.should raise_error(Thrift::TransportException) { |e| e.type.should == Thrift::TransportException::TIMED_OUT }
   end