diff --git a/lib/rb/lib/thrift/transport/server_socket.rb b/lib/rb/lib/thrift/transport/server_socket.rb index 60e4b8a105e..6fe69231d7d 100644 --- a/lib/rb/lib/thrift/transport/server_socket.rb +++ b/lib/rb/lib/thrift/transport/server_socket.rb @@ -38,6 +38,14 @@ def initialize(host_or_port, port = nil) def listen @handle = TCPServer.new(@host, @port) + + # Turn linger off, don't want to block on calls to close + begin + @handle.setsockopt(::Socket::SOL_SOCKET, ::Socket::SO_LINGER, [0, 0].pack('ii')) + rescue IOError => e + close + raise TransportException.new(TransportException::NOT_OPEN, "Could not set SO_LINGER: #{e.message}") + end end def accept diff --git a/lib/rb/lib/thrift/transport/socket.rb b/lib/rb/lib/thrift/transport/socket.rb index b3476ea5533..71f3fd21bad 100644 --- a/lib/rb/lib/thrift/transport/socket.rb +++ b/lib/rb/lib/thrift/transport/socket.rb @@ -28,15 +28,24 @@ def initialize(host='localhost', port=9090, timeout=nil) @timeout = timeout @desc = "#{host}:#{port}" @handle = nil + @linger_on = true + @linger_time = 0 end - attr_accessor :handle, :timeout + attr_accessor :handle, :timeout, :linger_on, :linger_time + + def linger(on, seconds) + @linger_on = on + @linger_time = seconds + apply_linger + end def open for addrinfo in ::Socket::getaddrinfo(@host, @port, nil, ::Socket::SOCK_STREAM) do begin socket = ::Socket.new(addrinfo[4], ::Socket::SOCK_STREAM, 0) socket.setsockopt(::Socket::IPPROTO_TCP, ::Socket::TCP_NODELAY, 1) + apply_linger(socket) sockaddr = ::Socket.sockaddr_in(addrinfo[1], addrinfo[3]) begin socket.connect_nonblock(sockaddr) @@ -139,5 +148,14 @@ def close def to_s "socket(#{@host}:#{@port})" end + + private + + def apply_linger(socket = @handle) + return if socket.nil? || socket.closed? + + option = [@linger_on ? 1 : 0, @linger_time].pack('ii') + socket.setsockopt(::Socket::SOL_SOCKET, ::Socket::SO_LINGER, option) + end end end diff --git a/lib/rb/lib/thrift/transport/ssl_server_socket.rb b/lib/rb/lib/thrift/transport/ssl_server_socket.rb index 3abd5ec3d24..db76039c341 100644 --- a/lib/rb/lib/thrift/transport/ssl_server_socket.rb +++ b/lib/rb/lib/thrift/transport/ssl_server_socket.rb @@ -30,8 +30,8 @@ def initialize(host_or_port, port = nil, ssl_context = nil) attr_accessor :ssl_context def listen - socket = TCPServer.new(@host, @port) - @handle = OpenSSL::SSL::SSLServer.new(socket, @ssl_context) + super + @handle = OpenSSL::SSL::SSLServer.new(@handle, @ssl_context) end def to_s diff --git a/lib/rb/spec/server_socket_spec.rb b/lib/rb/spec/server_socket_spec.rb index 56b3bac7c02..1f024dcc196 100644 --- a/lib/rb/spec/server_socket_spec.rb +++ b/lib/rb/spec/server_socket_spec.rb @@ -28,19 +28,22 @@ end it "should create a handle when calling listen" do - expect(TCPServer).to receive(:new).with(nil, 1234) + handle = double("TCPServer", :setsockopt => nil) + expect(TCPServer).to receive(:new).with(nil, 1234).and_return(handle) @socket.listen end it "should accept an optional host argument" do @socket = Thrift::ServerSocket.new('localhost', 1234) - expect(TCPServer).to receive(:new).with('localhost', 1234) + handle = double("TCPServer", :setsockopt => nil) + expect(TCPServer).to receive(:new).with('localhost', 1234).and_return(handle) @socket.to_s == "server(localhost:1234)" @socket.listen end it "should create a Thrift::Socket to wrap accepted sockets" do handle = double("TCPServer") + expect(handle).to receive(:setsockopt).with(Socket::SOL_SOCKET, Socket::SO_LINGER, [0, 0].pack('ii')) expect(TCPServer).to receive(:new).with(nil, 1234).and_return(handle) @socket.listen sock = double("sock") @@ -53,7 +56,7 @@ end it "should close the handle when closed" do - handle = double("TCPServer", :closed? => false) + handle = double("TCPServer", :closed? => false, :setsockopt => nil) expect(TCPServer).to receive(:new).with(nil, 1234).and_return(handle) @socket.listen expect(handle).to receive(:close) @@ -65,7 +68,7 @@ end it "should return true for closed? when appropriate" do - handle = double("TCPServer", :closed? => false) + handle = double("TCPServer", :closed? => false, :setsockopt => nil) allow(TCPServer).to receive(:new).and_return(handle) @socket.listen expect(@socket).not_to be_closed diff --git a/lib/rb/spec/socket_spec.rb b/lib/rb/spec/socket_spec.rb index 202c745ea2e..cd5b2ca6eea 100644 --- a/lib/rb/spec/socket_spec.rb +++ b/lib/rb/spec/socket_spec.rb @@ -40,19 +40,29 @@ end it "should open a ::Socket with default args" do - expect(::Socket).to receive(:new).and_return(double("Handle", :connect_nonblock => true, :setsockopt => nil)) + expect(::Socket).to receive(:new).and_return(double("Handle", :connect_nonblock => true, :setsockopt => nil, :closed? => false)) expect(::Socket).to receive(:getaddrinfo).with("localhost", 9090, nil, ::Socket::SOCK_STREAM).and_return([[]]) expect(::Socket).to receive(:sockaddr_in) - @socket.to_s == "socket(localhost:9090)" + expect(@socket.to_s).to eq("socket(localhost:9090)") + @socket.open + end + + it "should set linger on the socket before connecting" do + handle = double("Handle", :connect_nonblock => true, :closed? => false) + expect(::Socket).to receive(:new).and_return(handle) + expect(::Socket).to receive(:getaddrinfo).with("localhost", 9090, nil, ::Socket::SOCK_STREAM).and_return([[]]) + expect(handle).to receive(:setsockopt).with(Socket::SOL_SOCKET, Socket::SO_LINGER, [1, 0].pack('ii')) + expect(handle).to receive(:setsockopt).with(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) + expect(::Socket).to receive(:sockaddr_in) @socket.open end it "should accept host/port options" do - expect(::Socket).to receive(:new).and_return(double("Handle", :connect_nonblock => true, :setsockopt => nil)) + expect(::Socket).to receive(:new).and_return(double("Handle", :connect_nonblock => true, :setsockopt => nil, :closed? => false)) expect(::Socket).to receive(:getaddrinfo).with("my.domain", 1234, nil, ::Socket::SOCK_STREAM).and_return([[]]) expect(::Socket).to receive(:sockaddr_in) - @socket = Thrift::Socket.new('my.domain', 1234).open - @socket.to_s == "socket(my.domain:1234)" + @socket = Thrift::Socket.new('my.domain', 1234).tap(&:open) + expect(@socket.to_s).to eq("socket(my.domain:1234)") end it "should accept an optional timeout" do diff --git a/lib/rb/spec/ssl_server_socket_spec.rb b/lib/rb/spec/ssl_server_socket_spec.rb index 82e65184326..902f4311c6b 100644 --- a/lib/rb/spec/ssl_server_socket_spec.rb +++ b/lib/rb/spec/ssl_server_socket_spec.rb @@ -27,6 +27,14 @@ @socket = Thrift::SSLServerSocket.new(1234) end + it "should set linger on the underlying server socket" do + tcp = double("TCPServer") + expect(TCPServer).to receive(:new).with(nil, 1234).and_return(tcp) + expect(tcp).to receive(:setsockopt).with(Socket::SOL_SOCKET, Socket::SO_LINGER, [0, 0].pack('ii')) + expect(OpenSSL::SSL::SSLServer).to receive(:new).with(tcp, nil) + @socket.listen + end + it "should provide a reasonable to_s" do expect(@socket.to_s).to eq("ssl(socket(:1234))") end diff --git a/lib/rb/spec/ssl_socket_spec.rb b/lib/rb/spec/ssl_socket_spec.rb index 808d8d512ee..f52e2704b66 100644 --- a/lib/rb/spec/ssl_socket_spec.rb +++ b/lib/rb/spec/ssl_socket_spec.rb @@ -53,8 +53,18 @@ @socket.open end + it "should set linger on the underlying socket" do + expect(::Socket).to receive(:getaddrinfo).with("localhost", 9090, nil, ::Socket::SOCK_STREAM).and_return([[]]) + expect(::Socket).to receive(:sockaddr_in) + expect(@simple_socket_handle).to receive(:setsockopt).with(Socket::SOL_SOCKET, Socket::SO_LINGER, [1, 0].pack('ii')) + expect(@simple_socket_handle).to receive(:setsockopt).with(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) + expect(OpenSSL::SSL::SSLSocket).to receive(:new).with(@simple_socket_handle, nil).and_return(@handle) + expect(@handle).to receive(:post_connection_check).with('localhost') + @socket.open + end + it "should accept host/port options" do - handle = double("Handle", :connect_nonblock => true, :setsockopt => nil) + handle = double("Handle", :connect_nonblock => true, :setsockopt => nil, :closed? => false) allow(::Socket).to receive(:new).and_return(handle) expect(::Socket).to receive(:getaddrinfo).with("my.domain", 1234, nil, ::Socket::SOCK_STREAM).and_return([[]]) expect(::Socket).to receive(:sockaddr_in)