ruby-changes:50766
From: usa <ko1@a...>
Date: Wed, 28 Mar 2018 15:03:18 +0900 (JST)
Subject: [ruby-changes:50766] usa:r62949 (ruby_2_3): merge revision(s) 62671: [Backport #14571]
usa 2018-03-28 15:03:13 +0900 (Wed, 28 Mar 2018) New Revision: 62949 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=62949 Log: merge revision(s) 62671: [Backport #14571] resolv.rb: close socket * lib/resolv.rb (UnconnectedUDP#lazy_initialize): store new sockets before binding, so the sockets get closed when the requester is closing. * lib/resolv.rb (ConnectedUDP#lazy_initialize): ditto. * lib/resolv.rb (UnconnectedUDP#close): synchronize to get rid of race condition. * lib/resolv.rb (ConnectedUDP#close): ditto. [ruby-core:85901] [Bug #14571] From: quixoten (Devin Christensen) <quixoten@g...> Modified directories: branches/ruby_2_3/ Modified files: branches/ruby_2_3/ChangeLog branches/ruby_2_3/lib/resolv.rb branches/ruby_2_3/test/resolv/test_dns.rb branches/ruby_2_3/version.h Index: ruby_2_3/version.h =================================================================== --- ruby_2_3/version.h (revision 62948) +++ ruby_2_3/version.h (revision 62949) @@ -1,6 +1,6 @@ https://github.com/ruby/ruby/blob/trunk/ruby_2_3/version.h#L1 #define RUBY_VERSION "2.3.7" #define RUBY_RELEASE_DATE "2018-03-28" -#define RUBY_PATCHLEVEL 445 +#define RUBY_PATCHLEVEL 446 #define RUBY_RELEASE_YEAR 2018 #define RUBY_RELEASE_MONTH 3 Index: ruby_2_3/lib/resolv.rb =================================================================== --- ruby_2_3/lib/resolv.rb (revision 62948) +++ ruby_2_3/lib/resolv.rb (revision 62949) @@ -739,35 +739,47 @@ class Resolv https://github.com/ruby/ruby/blob/trunk/ruby_2_3/lib/resolv.rb#L739 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 + @initialized = false + @mutex = Thread::Mutex.new + 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, [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 = [host, port] @@ -779,9 +791,14 @@ class Resolv https://github.com/ruby/ruby/blob/trunk/ruby_2_3/lib/resolv.rb#L791 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 @@ -805,20 +822,32 @@ class Resolv https://github.com/ruby/ruby/blob/trunk/ruby_2_3/lib/resolv.rb#L822 super() @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) + @mutex = Thread::Mutex.new + @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,10 +858,15 @@ class Resolv https://github.com/ruby/ruby/blob/trunk/ruby_2_3/lib/resolv.rb#L858 end def close - super - @senders.each_key {|from, id| - DNS.free_request_id(@host, @port, id) - } + @mutex.synchronize do + if @initialized + super + @senders.each_key {|from, id| + DNS.free_request_id(@host, @port, id) + } + @initialized = false + end + end end class Sender < Requester::Sender # :nodoc: @@ -846,6 +880,7 @@ class Resolv https://github.com/ruby/ruby/blob/trunk/ruby_2_3/lib/resolv.rb#L880 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') @@ -855,6 +890,7 @@ class Resolv https://github.com/ruby/ruby/blob/trunk/ruby_2_3/lib/resolv.rb#L890 end def sender_for(addr, msg) + lazy_initialize @senders[msg.id] end end Index: ruby_2_3/test/resolv/test_dns.rb =================================================================== --- ruby_2_3/test/resolv/test_dns.rb (revision 62948) +++ ruby_2_3/test/resolv/test_dns.rb (revision 62949) @@ -3,6 +3,7 @@ require 'test/unit' https://github.com/ruby/ruby/blob/trunk/ruby_2_3/test/resolv/test_dns.rb#L3 require 'resolv' require 'socket' require 'tempfile' +require 'minitest/mock' class TestResolvDNS < Test::Unit::TestCase def setup @@ -221,4 +222,22 @@ class TestResolvDNS < Test::Unit::TestCa https://github.com/ruby/ruby/blob/trunk/ruby_2_3/test/resolv/test_dns.rb#L222 } assert_operator(2**14, :<, m.to_s.length) end + + def assert_no_fd_leak + socket = assert_throw(self) do |tag| + Resolv::DNS.stub(:bind_random_port, ->(s, *) {throw(tag, s)}) do + yield.getname("8.8.8.8") + end + end + + assert_predicate(socket, :closed?, "file descriptor leaked") + end + + def test_no_fd_leak_connected + assert_no_fd_leak {Resolv::DNS.new(nameserver_port: [['127.0.0.1', 53]])} + end + + def test_no_fd_leak_unconnected + assert_no_fd_leak {Resolv::DNS.new} + end end Index: ruby_2_3/ChangeLog =================================================================== --- ruby_2_3/ChangeLog (revision 62948) +++ ruby_2_3/ChangeLog (revision 62949) @@ -1,3 +1,22 @@ https://github.com/ruby/ruby/blob/trunk/ruby_2_3/ChangeLog#L1 +Thu Mar 28 15:02:43 2018 Nobuyoshi Nakada <nobu@r...> + + resolv.rb: close socket + + * lib/resolv.rb (UnconnectedUDP#lazy_initialize): store new + sockets before binding, so the sockets get closed when the + requester is closing. + + * lib/resolv.rb (ConnectedUDP#lazy_initialize): ditto. + + * lib/resolv.rb (UnconnectedUDP#close): synchronize to get rid of + race condition. + + * lib/resolv.rb (ConnectedUDP#close): ditto. + + [Bug #14571] + + From: quixoten (Devin Christensen) quixoten@g... + Thu Mar 28 14:59:27 2018 Nobuyoshi Nakada <nobu@r...> socket.c: null byte at Socket.getnameinfo Index: ruby_2_3 =================================================================== --- ruby_2_3 (revision 62948) +++ ruby_2_3 (revision 62949) Property changes on: ruby_2_3 ___________________________________________________________________ Modified: svn:mergeinfo ## -0,0 +0,1 ## Merged /trunk:r62671 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/