summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authornobu <nobu@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2018-03-06 02:48:17 +0000
committernobu <nobu@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2018-03-06 02:48:17 +0000
commite6c0a8f19e3f38fdc77906ea545f8d3b0479adf7 (patch)
tree43ebf72cd70c35a3b6c96a0cffbcbbe8d59e899b
parent4b747c2890940dddb226cdc3f59acc77725a9c6e (diff)
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@gmail.com> git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@62671 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
-rw-r--r--lib/resolv.rb102
-rw-r--r--test/resolv/test_dns.rb19
2 files changed, 88 insertions, 33 deletions
diff --git a/lib/resolv.rb b/lib/resolv.rb
index 830ba16..e296516 100644
--- a/lib/resolv.rb
+++ b/lib/resolv.rb
@@ -732,35 +732,47 @@ class Resolv
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]
@@ -772,9 +784,14 @@ class Resolv
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
@@ -798,20 +815,32 @@ class Resolv
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
@@ -822,10 +851,15 @@ class Resolv
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:
@@ -839,6 +873,7 @@ class Resolv
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')
@@ -848,6 +883,7 @@ class Resolv
end
def sender_for(addr, msg)
+ lazy_initialize
@senders[msg.id]
end
end
diff --git a/test/resolv/test_dns.rb b/test/resolv/test_dns.rb
index 1b44f32..1ac3aea 100644
--- a/test/resolv/test_dns.rb
+++ b/test/resolv/test_dns.rb
@@ -3,6 +3,7 @@ require 'test/unit'
require 'resolv'
require 'socket'
require 'tempfile'
+require 'minitest/mock'
class TestResolvDNS < Test::Unit::TestCase
def setup
@@ -246,4 +247,22 @@ class TestResolvDNS < Test::Unit::TestCase
}
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