Merge branch 'master' into ruby-2.5

jruby · Mar 20, 2018 · afa651d · afa651d
2 parents 3cf3392 + 3813f0a
commit afa651d
Showing 2 changed files with 109 additions and 32 deletions.
diff --git a/lib/ruby/stdlib/resolv.rb b/lib/ruby/stdlib/resolv.rb
@@ -727,6 +727,10 @@ def close
         socks&.each(&:close)
       end
 
+      def lazy_initialize
+        self
+      end
+
       class Sender # :nodoc:
         def initialize(msg, data, sock)
           @msg = msg
@@ -739,35 +743,47 @@ class UnconnectedUDP < Requester # :nodoc:
         def initialize(*nameserver_port)
           super()
           @nameserver_port = nameserver_port
-          @socks_hash = {}
-          @socks = []
-          nameserver_port.each {|host, port|
-            if host.index(':')
-              bind_host = "::"
-              af = Socket::AF_INET6
-            else
-              bind_host = "0.0.0.0"
-              af = Socket::AF_INET
-            end
-            next if @socks_hash[bind_host]
-            begin
-              sock = UDPSocket.new(af)
-            rescue Errno::EAFNOSUPPORT
-              next # The kernel doesn't support the address family.
-            end
-            sock.do_not_reverse_lookup = true
-            DNS.bind_random_port(sock, bind_host)
-            @socks << sock
-            @socks_hash[bind_host] = sock
+          @mutex = Thread::Mutex.new
+          @initialized = false
+        end
+
+        def lazy_initialize
+          @mutex.synchronize {
+            next if @initialized
+            @initialized = true
+            @socks_hash = {}
+            @socks = []
+            @nameserver_port.each {|host, port|
+              if host.index(':')
+                bind_host = "::"
+                af = Socket::AF_INET6
+              else
+                bind_host = "0.0.0.0"
+                af = Socket::AF_INET
+              end
+              next if @socks_hash[bind_host]
+              begin
+                sock = UDPSocket.new(af)
+              rescue Errno::EAFNOSUPPORT
+                next # The kernel doesn't support the address family.
+              end
+              @socks << sock
+              @socks_hash[bind_host] = sock
+              sock.do_not_reverse_lookup = true
+              DNS.bind_random_port(sock, bind_host)
+            }
           }
+          self
         end
 
         def recv_reply(readable_socks)
+          lazy_initialize
           reply, from = readable_socks[0].recvfrom(UDPSize)
           return reply, [IPAddr.new(from[3]),from[1]]
         end
 
         def sender(msg, data, host, port=Port)
+          lazy_initialize
           sock = @socks_hash[host.index(':') ? "::" : "0.0.0.0"]
           return nil if !sock
           service = [IPAddr.new(host), port]
@@ -779,9 +795,14 @@ def sender(msg, data, host, port=Port)
         end
 
         def close
-          super
-          @senders.each_key {|service, id|
-            DNS.free_request_id(service[0], service[1], id)
+          @mutex.synchronize {
+            if @initialized
+              super
+              @senders.each_key {|service, id|
+                DNS.free_request_id(service[0], service[1], id)
+              }
+              @initialized = false
+            end
           }
         end
 
@@ -803,22 +824,34 @@ def send
       class ConnectedUDP < Requester # :nodoc:
         def initialize(host, port=Port)
           super()
+          @mutex = Thread::Mutex.new
           @host = host
           @port = port
-          is_ipv6 = host.index(':')
-          sock = UDPSocket.new(is_ipv6 ? Socket::AF_INET6 : Socket::AF_INET)
-          @socks = [sock]
-          sock.do_not_reverse_lookup = true
-          DNS.bind_random_port(sock, is_ipv6 ? "::" : "0.0.0.0")
-          sock.connect(host, port)
+          @initialized = false
+        end
+
+        def lazy_initialize
+          @mutex.synchronize {
+            next if @initialized
+            @initialized = true
+            is_ipv6 = @host.index(':')
+            sock = UDPSocket.new(is_ipv6 ? Socket::AF_INET6 : Socket::AF_INET)
+            @socks = [sock]
+            sock.do_not_reverse_lookup = true
+            DNS.bind_random_port(sock, is_ipv6 ? "::" : "0.0.0.0")
+            sock.connect(@host, @port)
+          }
+          self
         end
 
         def recv_reply(readable_socks)
+          lazy_initialize
           reply = readable_socks[0].recv(UDPSize)
           return reply, nil
         end
 
         def sender(msg, data, host=@host, port=@port)
+          lazy_initialize
           unless host == @host && port == @port
             raise RequestError.new("host/port don't match: #{host}:#{port}")
           end
@@ -829,9 +862,14 @@ def sender(msg, data, host=@host, port=@port)
         end
 
         def close
-          super
-          @senders.each_key {|from, id|
-            DNS.free_request_id(@host, @port, id)
+          @mutex.synchronize {
+            if @initialized
+              super
+              @senders.each_key {|from, id|
+                DNS.free_request_id(@host, @port, id)
+              }
+              @initialized = false
+            end
           }
         end
 
@@ -846,6 +884,7 @@ def send
 
       class MDNSOneShot < UnconnectedUDP # :nodoc:
         def sender(msg, data, host, port=Port)
+          lazy_initialize
           id = DNS.allocate_request_id(host, port)
           request = msg.encode
           request[0,2] = [id].pack('n')

diff --git a/test/mri/resolv/test_dns.rb b/test/mri/resolv/test_dns.rb
@@ -3,6 +3,8 @@
 require 'resolv'
 require 'socket'
 require 'tempfile'
+require 'timeout'
+require 'minitest/mock'
 
 class TestResolvDNS < Test::Unit::TestCase
   def setup
@@ -246,4 +248,40 @@ def test_too_big_label_address
     }
     assert_operator(2**14, :<, m.to_s.length)
   end
+
+  def test_timeout_without_leaking_file_descriptors_connected
+    socket = nil
+    bind_random_port = lambda do |udpsock, bind_host="0.0.0.0"|
+      socket = udpsock
+      sleep 3
+    end
+
+    Resolv::DNS.stub(:bind_random_port, bind_random_port) do
+      r = Resolv::DNS.new(nameserver_port: [['127.0.0.1', 53]])
+      begin
+        Timeout.timeout(0.5) { r.getname("8.8.8.8") }
+      rescue Timeout::Error
+      end
+    end
+
+    assert(socket.closed?, "file descriptor leaked")
+  end
+
+  def test_timeout_without_leaking_file_descriptors_unconnected
+    socket = nil
+    bind_random_port = lambda do |udpsock, bind_host="0.0.0.0"|
+      socket = udpsock
+      sleep 3
+    end
+
+    Resolv::DNS.stub(:bind_random_port, bind_random_port) do
+      r = Resolv::DNS.new
+      begin
+        Timeout.timeout(0.5) { r.getname("8.8.8.8") }
+      rescue Timeout::Error
+      end
+    end
+
+    assert(socket.closed?, "file descriptor leaked")
+  end
 end